Skip to content

Emoji picker portaling#7217

Merged
gaearon merged 7 commits into
mainfrom
emoji-picker-portaling
Dec 20, 2024
Merged

Emoji picker portaling#7217
gaearon merged 7 commits into
mainfrom
emoji-picker-portaling

Conversation

@estrattonbailey
Copy link
Copy Markdown
Member

@estrattonbailey estrattonbailey commented Dec 20, 2024

The emoji picker within the composer wasn't allowing users to focus the input, although the individual buttons still worked. This was due to the FocusScope wrapping the Composer, which prevents focus from leaving the composer dialog.

In #7146, I wrapped the picker in Portal so that its positioning was independent of wrapping elements and their styles. This had the unintentional result of portal-ing the picker outside the FocusScope, meaning focus couldn't be applied to the picker's search input.

The fix here is to wrap the picker in Portal only when needed, which is currently just on the Conversation screen. Instead of doing this in situ (first commit), I've added a portal?: boolean prop to the picker that optionally wraps it in a Portal. We fixed this an explicit nested FocusScope instead.

Testing: make sure all features of picker are accessible in both the composer and the conversation screen.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 20, 2024

Old size New size Diff
6.84 MB 6.84 MB 544 B (0.01%)

@arcalinea arcalinea temporarily deployed to emoji-picker-portaling - social-app PR #7217 December 20, 2024 19:41 — with Render Destroyed
@gaearon gaearon merged commit 8116d12 into main Dec 20, 2024
@gaearon gaearon deleted the emoji-picker-portaling branch December 20, 2024 20:02
Signez pushed a commit to Signez/bsky-social-app that referenced this pull request Dec 26, 2024
* Only portal the emoji picker where needed

* Add optional portal prop to emoji picker

* Use FocusScope to our advantage

* Pare back, add guards, fix focus trap

* Don't return focus to emoji button

* Set DM input position on emoji insert

* Let the caller determine next focus node

---------

Co-authored-by: Dan Abramov <dan.abramov@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants