-
-
Notifications
You must be signed in to change notification settings - Fork 4.9k
feat: revamp at menu #8016
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: revamp at menu #8016
Conversation
Reviewer's GuideThis PR overhauls the “@” mention menu by refactoring inline action handlers into reusable EditorState extensions, consolidating menu positioning, enhancing UI components, adding persistent toggle state, and introducing full person-mention support with dedicated BLoCs and UI widgets. Sequence Diagram for Mentioning a PersonsequenceDiagram
actor User
participant Editor
participant CommandHandler as inlineActionsCommandHandler
participant MenuService as MentionMenuService
participant MentionUI as MentionMenu (UI)
participant MentionBLoC as MentionBloc
participant EditorExt as EditorState Extensions
User->>Editor: Types "@"
Editor->>CommandHandler: Triggers with "@"
CommandHandler->>MenuService: new MentionMenuService(...)
MenuService->>MenuService: show()
MenuService->>MentionUI: Creates and displays
User->>MentionUI: Types person's name (e.g., "And")
MentionUI->>MentionBLoC: add(MentionEvent.query("And"))
MentionBLoC->>MentionBLoC: Filters/fetches persons
MentionBLoC-->>MentionUI: Updates state with filtered persons
MentionUI-->>User: Shows filtered list (e.g., Andrew C., Andrew T.)
User->>MentionUI: Selects "Andrew C."
MentionUI->>EditorExt: insertPerson(Person("Andrew C."), ...)
activate EditorExt
EditorExt->>EditorExt: buildMentionPersonAttributes(...)
EditorExt->>Editor: apply(transaction with mention)
deactivate EditorExt
Editor-->>User: Displays "@Andrew C."
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @asjqkkkk - I've reviewed your changes - here's some feedback:
- Extract the duplicated overlay positioning logic in InlineActionsMenu and MentionMenuService into a shared helper or base class to reduce code duplication.
- Consolidate the various EditorState extension methods into a single extension file or folder to improve discoverability and maintainability.
Here's what I looked at during the review
- 🟡 General issues: 4 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
frontend/appflowy_flutter/lib/features/mension_person/presentation/widgets/person_list.dart
Show resolved
Hide resolved
...flowy_flutter/lib/features/mension_person/presentation/widgets/item_visibility_detector.dart
Show resolved
Hide resolved
frontend/appflowy_flutter/lib/features/mension_person/logic/mention_bloc.dart
Outdated
Show resolved
Hide resolved
frontend/appflowy_flutter/lib/features/mension_person/presentation/mention_menu_service.dart
Outdated
Show resolved
Hide resolved
69585b6 to
1302af0
Compare
| }); | ||
|
|
||
| final String email; | ||
| final PersonRole role; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the ShareRole before, we can reuse it and rename it to WorkspaceUserRole.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we add "contact" to ShareRole?
frontend/appflowy_flutter/lib/features/mension_person/data/repositories/mention_repository.dart
Outdated
Show resolved
Hide resolved
frontend/appflowy_flutter/lib/features/mension_person/logic/mention_bloc.dart
Outdated
Show resolved
Hide resolved
frontend/appflowy_flutter/lib/features/mension_person/logic/mention_bloc.dart
Outdated
Show resolved
Hide resolved
frontend/appflowy_flutter/lib/features/mension_person/logic/mention_bloc.dart
Outdated
Show resolved
Hide resolved
frontend/appflowy_flutter/lib/features/mension_person/logic/person_bloc.dart
Outdated
Show resolved
Hide resolved
frontend/appflowy_flutter/lib/features/mension_person/logic/person_bloc.dart
Outdated
Show resolved
Hide resolved
frontend/appflowy_flutter/lib/features/mension_person/logic/person_bloc.dart
Outdated
Show resolved
Hide resolved
...nd/appflowy_flutter/lib/features/mension_person/presentation/widgets/date_reminder_list.dart
Outdated
Show resolved
Hide resolved
...owy_flutter/lib/features/mension_person/presentation/widgets/invite/contact_detail_menu.dart
Outdated
Show resolved
Hide resolved
...nd/appflowy_flutter/lib/features/mension_person/presentation/widgets/invite/invite_menu.dart
Outdated
Show resolved
Hide resolved
| itemMap.addToPerson( | ||
| MentionMenuItem(id: id, onExecute: () => invitePerson(context)), | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it intentional to call this function in the build function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it's used to implement the feature: selecte items by arrow key
...flutter/lib/features/mension_person/presentation/widgets/invite/person_list_invite_item.dart
Outdated
Show resolved
Hide resolved
| import 'package:flutter_bloc/flutter_bloc.dart'; | ||
| import 'package:visibility_detector/visibility_detector.dart'; | ||
|
|
||
| class MentionMenuItenVisibilityDetector extends StatelessWidget { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of this widget?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is for implementing selecting items using arrow keys
...ppflowy_flutter/lib/features/mension_person/presentation/widgets/mention_menu_shortcuts.dart
Show resolved
Hide resolved
...flowy_flutter/lib/features/mension_person/presentation/widgets/person/person_role_badge.dart
Outdated
Show resolved
Hide resolved
frontend/appflowy_flutter/packages/appflowy_ui/lib/src/component/menu/menu.dart
Outdated
Show resolved
Hide resolved
91f5df8 to
4faafd3
Compare
🥷 Ninja i18n – 🛎️ Translations need to be updatedProject
|
| lint rule | new reports | level | link |
|---|---|---|---|
| Missing translation | 1500 | warning | contribute (via Fink 🐦) |
b76e3a0 to
666fa88
Compare
666fa88 to
ce2c65f
Compare
d9296c3 to
ab2d2e4
Compare
Feature Preview
PR Checklist
Summary by Sourcery
Revamp the inline “@” mention menu to support person, page, date, and reminder mentions with a unified new menu service and modular callbacks. Extract insertion logic into EditorState extension methods and streamline the inline menu’s positioning and rendering. Introduce a dedicated MentionMenuService with Bloc-driven lists for people, pages, and date/reminder items, including a persistent “send notification” toggle and mock data providers for person mentions. Enhance AFMenu, AFMenuSection, and item widgets to allow customizable builders, trailing widgets, and background color hooks.
New Features:
Enhancements:
Chores: