feat(dm): wire full emoji picker for custom reactions by realmeylisdev · Pull Request #4711 · divinevideo/divine-mobile

realmeylisdev · 2026-05-25T08:25:39Z

Description

The + button in the DM reaction long-press row was intended to open a full emoji picker, but its caller handler was a no-op stub (TODO(#4633)): tapping it just dismissed the sheet and did nothing (#4710). Per @hm21's review, this PR wires it up rather than removing it.

Added FullReactionEmojiPickerSheet — a Divine dark-styled bottom sheet wrapping pro_image_editor's built-in EmojiEditor, which is already a direct dependency for the video editor. No new picker package.
conversation_view._onMessageLongPress now opens the picker on + and dispatches the picked emoji through the existing ConversationReactionToggled path (extracted a shared _toggleReaction helper used by both the quick row and the full picker).
No repository or protocol work: the NIP-17 wrapped-reaction plumbing from feat(dm): emoji reactions on messages #4633 already accepts arbitrary emoji, so a custom emoji flows through unchanged.

This supersedes the earlier "temporarily remove the + button" approach. Making the path live also clears the dead openFullPicker seam, the stale TODO(#4633) breadcrumbs (that issue is closed), and re-uses the previously-orphaned dmReactionAddCustomA11yLabel ARB key — resolving all three self-review threads by construction.

Out of Scope

Theming the picker's secondary bars beyond the main grid background — the package's dark defaults are used for the bottom action / search bar.
A widget test that taps a specific emoji in the third-party grid and asserts the dispatched event: brittle across pro_image_editor versions. The picker-opens and dismiss→null paths are tested; the final dispatch hop reuses the (tested) quick-reaction _toggleReaction helper.

Verification

flutter analyze lib test integration_test — No issues found
dart format <changed files> — clean (0 changed)
flutter test test/screens/inbox/conversation/ — 119/119 passing, including:
- new full_reaction_emoji_picker_sheet_test.dart (picker mounts; dismiss → null)
- strengthened reaction_picker_overlay_test.dart (+ pops openFullPicker: true)
- new end-to-end conversation_view_test.dart case (long-press → + → EmojiEditor opens)

Type of Change

✨ New feature (non-breaking change which adds functionality)
🛠️ Bug fix (non-breaking change which fixes an issue)
❌ Breaking change (fix or feature that would cause existing functionality to change)
🧹 Code refactor
✅ Build configuration change
📝 Documentation
🗑️ Chore

Related Issue: Closes #4710 — completes the full-picker integration deferred by #4633

realmeylisdev · 2026-05-25T08:44:33Z

Simulator Screenshot - iPhone 17 Pro Max - 2026-05-25 at 13 18 22

hm21

@realmeylisdev LGTM, but I’m wondering if we should just use a basic emoji picker for now instead of removing it temporarily. I don’t think it would have much negative impact on the design, while the usability benefit could be pretty valuable.

We already use thepro_image_editor package for the editor, and it actually includes an emoji picker out of the box. Below is a quick example. WDYT?

   EmojiLayer? layer = await showModalBottomSheet(
      context: context,
      isScrollControlled: true,
      useSafeArea: true,
      builder: (BuildContext context) => SafeArea(
        child: ConstrainedBox(
          constraints: BoxConstraints(
            maxHeight: 300 + MediaQuery.viewInsetsOf(context).bottom,
          ),
          child: EmojiEditor(),
        ),
      ),
    );

realmeylisdev

Review: Approve-with-comments (non-blocking).

The change is correct and safe: it does exactly what it claims, all CI is green, the public show() API is unchanged, and the tests were updated honestly to match. The inline notes below are code-health / process items, not correctness blockers.

Note: I'm authenticated here as the PR author, and GitHub doesn't allow approving your own PR — so this is submitted as a Comment review. The formal Approve needs to come from a second reviewer (e.g. @hm21).

Two things worth resolving before merge:

The preserved openFullPicker seam is now unreachable dead code, and its TODO(#4633) breadcrumbs point at a closed issue (#4633) — with #4710 closing on merge, restoration ends up tracked by nothing. Either drop the seam (it's ~4 trivial lines to reshape back later) or repoint the TODOs at a live issue. See inline.
Strategic (from @hm21): pro_image_editor is already a direct dependency and exports a standalone EmojiEditor that pops EmojiLayer(emoji: ...). A picked emoji flows through the existing ConversationReactionToggled path unchanged — no new dependency, no repository/protocol work (the NIP-17 wrapped-reaction plumbing from #4633 already accepts arbitrary emoji). If there's product appetite, wiring the picker now may beat a deferral. Doesn't block this PR.

realmeylisdev · 2026-05-25T11:51:08Z

Simulator Screenshot - iPhone 17 Pro Max - 2026-05-25 at 16 50 01

The "+" affordance in the DM reaction long-press row returned ReactionPickerResult(openFullPicker: true) into a no-op caller stub — tapping it dismissed the sheet and did nothing (#4710). Wire it to pro_image_editor's built-in EmojiEditor (already a direct dependency for the video editor) via a new FullReactionEmojiPickerSheet, shown as a Divine dark-styled bottom sheet. The picked emoji flows through the existing ConversationReactionToggled path unchanged — no new dependency and no repository/protocol work, since the NIP-17 wrapped-reaction plumbing already accepts arbitrary emoji. This supersedes the earlier "temporarily remove the + button" approach per review on #4711, and clears the dead openFullPicker seam and the stale TODO(#4633) breadcrumbs by making the path live. Closes #4710

realmeylisdev · 2026-05-25T12:01:43Z

Went with @hm21's suggestion — wired up pro_image_editor's built-in EmojiEditor instead of removing the + button. A new dark-styled FullReactionEmojiPickerSheet opens on +, and the picked emoji flows through the existing ConversationReactionToggled path (no new dependency, no repository/protocol work — the NIP-17 wrapped-reaction plumbing already accepts arbitrary emoji).

Making the path live resolves the three self-review threads by construction:

Dead openFullPicker seam → now reachable (the + sets it, the handler consumes it).
Stale TODO(#4633) → removed; the deferred work is done.
Orphaned dmReactionAddCustomA11yLabel → used again by the restored + affordance.

The branch was force-pushed (rebased on fresh origin/main) so the diff is now a clean additive feature rather than a remove-then-restore. flutter analyze lib test integration_test clean; flutter test test/screens/inbox/conversation/ 119/119. Still Closes #4710.

realmeylisdev · 2026-05-25T12:33:24Z

@hm21 this is ready for a formal review when you have a moment. ✅ All CI green (Mobile CI, Tests, Analyze, Format, Generated Files, web preview), and all three review threads are resolved.

It's the version that wires up your EmojiEditor suggestion — tapping + now opens a Divine dark-styled FullReactionEmojiPickerSheet, and the picked emoji flows through the existing ConversationReactionToggled path. Since you're the second reviewer, your Approve is the only remaining gate (the author can't self-approve).

Heads-up on the earlier red Tests runs: that was a pre-existing flaky test unrelated to this diff — video_notification_row_test.dart "does not overflow at large text sizes" (passes locally, fails intermittently on Linux CI, and has flaked on other branches too). It cleared on re-run with no code change, and I filed #4719 to harden it separately.

hm21

Thanks @realmeylisdev for the update, love to see it include now the emoji-picker from my package ;)
I left some small comments for some details but in general it looks good already.

hm21 · 2026-05-25T14:21:55Z

+  /// Dark-mode editor configuration. Matches the picker grid background to
+  /// the surrounding sheet so the surface reads as a single dark panel.
+  static const _editorConfigs = ProImageEditorConfigs(
+    emojiEditor: EmojiEditorConfigs(


we should also ensure the categories are translated. You can do that in this configs like below

i18n: I18n( emojiEditor: I18nEmojiEditor( categoryActivities: '...', ) ),

Done in 92e9e79. Wired I18nEmojiEditor to context.l10n through a @visibleForTesting buildI18n(AppLocalizations) helper — added 10 ARB keys (9 category titles + the search hint) and translated them across all 18 locales. Went the l10n route rather than inline strings because Divine's rules don't allow hardcoded English in widgets, and I18nEmojiEditor defaults to English. A unit test asserts the label mapping and that it follows the active locale (en vs de).

hm21 · 2026-05-25T14:30:30Z

+      style: EmojiEditorStyle(
+        backgroundColor: VineTheme.surfaceBackground,
+      ),


I would recommend adding this style configs too

EmojiEditorStyle( backgroundColor: VineTheme.surfaceBackground, searchViewConfig: SearchViewConfig( backgroundColor: VineTheme.surfaceBackground, buttonIconColor: VineTheme.onSurface, ), bottomActionBarConfig: BottomActionBarConfig( backgroundColor: VineTheme.surfaceBackground, buttonColor: VineTheme.surfaceBackground, showBackspaceButton: false, ), categoryViewConfig: CategoryViewConfig( backgroundColor: VineTheme.surfaceBackground, ), emojiViewConfig: EmojiViewConfig( backgroundColor: VineTheme.surfaceBackground, ), ),

Done in 92e9e79. Added all four sub-configs — searchViewConfig, bottomActionBarConfig, categoryViewConfig, emojiViewConfig — on VineTheme.surfaceBackground, with buttonIconColor: VineTheme.onSurface and showBackspaceButton: false. Good catch: those three view configs default to light grey (0xFFEBEFF2) when null, so the search/category/grid surfaces were visibly off against the dark sheet. This pulls the secondary-bar theming I'd originally marked out-of-scope back into the PR.

The "+" affordance in the DM reaction long-press row returned ReactionPickerResult(openFullPicker: true) into a no-op caller stub — tapping it dismissed the sheet and did nothing (#4710). Wire it to pro_image_editor's built-in EmojiEditor (already a direct dependency for the video editor) via a new FullReactionEmojiPickerSheet, shown as a Divine dark-styled bottom sheet. The picked emoji flows through the existing ConversationReactionToggled path unchanged — no new dependency and no repository/protocol work, since the NIP-17 wrapped-reaction plumbing already accepts arbitrary emoji. This supersedes the earlier "temporarily remove the + button" approach per review on #4711, and clears the dead openFullPicker seam and the stale TODO(#4633) breadcrumbs by making the path live. Closes #4710

@hm21

Address @hm21's review on #4711: - Localize the picker's category / search labels via I18nEmojiEditor, wired to context.l10n. pro_image_editor defaults these to hardcoded English; add 10 ARB keys (9 categories + search hint) with real translations across all 18 locales. - Dark-theme every picker sub-surface (search bar, category bar, emoji grid, bottom action bar) with VineTheme.surfaceBackground. The package defaults searchView/categoryView/emojiView to light grey, which stood out against the dark sheet. - Present via VineBottomSheet instead of a raw showModalBottomSheet so the sheet inherits Divine chrome (rounded corners, drag handle, surface color). Fixed mode preserves the ~320px height and avoids nesting EmojiEditor's internal scroll inside a draggable sheet. Adds a @VisibleForTesting buildI18n covered by a unit test that asserts the label mapping and locale sensitivity.

realmeylisdev · 2026-05-25T17:44:14Z

@hm21 thanks for the review — all three are addressed in 92e9e79:

Categories localized — I18nEmojiEditor is now built from context.l10n; 10 new ARB keys (9 category titles + search hint) translated across all 18 locales.
Dark theming — searchViewConfig / bottomActionBarConfig / categoryViewConfig / emojiViewConfig all painted with VineTheme.surfaceBackground (they defaulted to light grey).
VineBottomSheet — swapped in, replacing the raw showModalBottomSheet. One intentional deviation: scrollable: false (fixed mode) to preserve the ~320px height and avoid nesting the picker's scroll inside a draggable sheet — details in that thread.

flutter analyze lib test integration_test clean; test/screens/inbox/conversation/ green at 121/121 (added a buildI18n unit test). Updated dark-mode screenshot to follow once I grab it on device.

github-actions · 2026-05-25T17:46:02Z

Mobile PR Preview

Preview refreshed for 92e9e79

Last refresh: 92e9e79 at 2026-05-25 17:46:01 UTC (preview run)

Property	Value
Preview URL	https://551f4815.openvine-app.pages.dev
Pages project	`openvine-app`
Preview branch	`pr-4711`
PR branch	`fix/dm-reaction-hide-custom-picker-button`
Commit	`92e9e79`

realmeylisdev · 2026-05-25T17:50:28Z

Simulator Screenshot - iPhone 17 Pro Max - 2026-05-25 at 22 48 03

Simulator Screenshot - iPhone 17 Pro Max - 2026-05-25 at 22 47 55

hm21

LGTM

realmeylisdev requested a review from hm21 May 25, 2026 08:26

realmeylisdev mentioned this pull request May 25, 2026

DM reaction picker: the "+" (custom emoji) button is a no-op #4710

Closed

This comment has been minimized.

Sign in to view

hm21 reviewed May 25, 2026

View reviewed changes

realmeylisdev commented May 25, 2026

View reviewed changes

Comment thread mobile/lib/screens/inbox/conversation/conversation_view.dart

Comment thread mobile/lib/screens/inbox/conversation/widgets/reaction_picker_overlay.dart

Comment thread mobile/lib/screens/inbox/conversation/widgets/reaction_picker_overlay.dart Outdated

realmeylisdev force-pushed the fix/dm-reaction-hide-custom-picker-button branch from 41ce1ab to 92b51b3 Compare May 25, 2026 12:00

realmeylisdev changed the title ~~fix(dm): temporarily remove custom-reaction "+" button~~ feat(dm): wire full emoji picker for custom reactions May 25, 2026

This comment has been minimized.

Sign in to view

realmeylisdev mentioned this pull request May 25, 2026

fix(notifications): flaky video_notification_row large-text overflow test on CI #4719

Closed

realmeylisdev requested a review from hm21 May 25, 2026 13:24

hm21 reviewed May 25, 2026

View reviewed changes

realmeylisdev added 2 commits May 25, 2026 22:41

realmeylisdev force-pushed the fix/dm-reaction-hide-custom-picker-button branch from 92b51b3 to 92e9e79 Compare May 25, 2026 17:42

realmeylisdev requested a review from hm21 May 25, 2026 17:51

hm21 approved these changes May 26, 2026

View reviewed changes

realmeylisdev merged commit 48a15a8 into main May 26, 2026
10 checks passed

realmeylisdev deleted the fix/dm-reaction-hide-custom-picker-button branch May 26, 2026 04:03

Conversation

realmeylisdev commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Out of Scope

Verification

Type of Change

Uh oh!

This comment has been minimized.

realmeylisdev commented May 25, 2026

Uh oh!

hm21 left a comment

Choose a reason for hiding this comment

Uh oh!

realmeylisdev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

realmeylisdev commented May 25, 2026

Uh oh!

realmeylisdev commented May 25, 2026

Uh oh!

This comment has been minimized.

realmeylisdev commented May 25, 2026

Uh oh!

hm21 left a comment

Choose a reason for hiding this comment

Uh oh!

hm21 May 25, 2026

Choose a reason for hiding this comment

Uh oh!

realmeylisdev May 25, 2026

Choose a reason for hiding this comment

Uh oh!

hm21 May 25, 2026

Choose a reason for hiding this comment

Uh oh!

realmeylisdev May 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

realmeylisdev commented May 25, 2026

Uh oh!

github-actions Bot commented May 25, 2026

Mobile PR Preview

Uh oh!

realmeylisdev commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hm21 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

realmeylisdev commented May 25, 2026 •

edited

Loading

realmeylisdev commented May 25, 2026 •

edited

Loading