-
Notifications
You must be signed in to change notification settings - Fork 93
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
enh(NcEmojiPicker): Always show skin tone selector + save selection #5103
Conversation
…n colors Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
6cce23e
to
56c6945
Compare
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.
Code changes look good. Did not try it out though.
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.
Very nice! 2 questions:
- Since there are 5 modifiers + default yellow, could the color dropdown be a 32 grid instead of 42 with only 2 in the bottom row? That would look cleaner. :)
- Once this color is set, will it also be used in cases where you use
:
for emojis like in Text, Talk, Collectives, Mail, etc?
Thanks!
Also save the selected skin tone for future uses and communicate it with all other NcEmojiPicker instances Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
56c6945
to
b2527dd
Compare
This comes from NcColorPicker and should probably be fixed there, would do so in a follow up. |
@susnux sounds good! :)
What about this one? Also cc @max-nextcloud for Text & Collectives, maybe you know? |
I need to test this but I think we need to adjust the |
…and use in `EmojiSearch` Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
No it did not work, but adjusting So @jancborchardt all points resolved and working 🎉 vokoscreenNG-2024-01-23_15-35-35.mp4 |
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.
Nice, awesome job @susnux! :)
@trailing-button-click="clearSearch(); slotProps.onSearch(search);" | ||
@update:value="slotProps.onSearch(search)" /> | ||
<NcColorPicker palette-only | ||
:palette="skinTonePalette" |
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.
Missing a container here for inner NcPopover. Because of that, skin tones are rendered always on 'body'
☑️ Resolves
Always allow to select the skin tone used for emojis, this can be done by picking the skin tone on the left of the search bar.
The selected skin tone is saved on the local storage of the browser, so it is preserved during sessions.
Previously it was only possible to select a skin tone if the preview was enabled, but this has two problems:
Some minor other changes on components to implement this:
Color
class so we can reuse itcolor
attribute required by the color picker (otherwise one would have to provide it manually)🖼️ Screenshots
vokoscreenNG-2024-01-22_01-32-48.mp4
🚧 Tasks
skin
property changes serebrov/emoji-mart-vue#294🏁 Checklist