-
-
Notifications
You must be signed in to change notification settings - Fork 166
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
add usability improvements to sticker selector UI #3293
Conversation
) | ||
})} | ||
{stickerPackImages.map((filePath, index) => ( | ||
<button |
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 I add an aria-label for this?
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.
with the filename? or what did you have in mind here?
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.
don't have anything specific in mind, but yeah maybe something like "Send FILENAME" would work?
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.
changelog still needs an entry about this, otherwise code looks good.
Works fine.
"Add some stickers bellow" does not tell how to do it, maybe we need a more detailed explanation (about add a folder, name is name of pack and content is image files)? @r10s what do you think translation wise?
Also stickers don't refresh when I change something with the folder until I close and reopen the whole emoji&sticker popup, but that minor issue is out of scope here.
) | ||
})} | ||
{stickerPackImages.map((filePath, index) => ( | ||
<button |
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.
with the filename? or what did you have in mind here?
yeah agreed. definitely one of the things I intended to update based on review :)
yeah i noticed that too and was tempted to figure out how that could work, but agree that it's out of scope for this (simplest thing would be to start polling when that panel is open I guess)
will update this after I address last remaining feedback 👍 |
_locales/_untranslated_en.json
Outdated
"message": "No stickers yet" | ||
}, | ||
"add_stickers_instructions": { | ||
"message": "Add some using the button below." |
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.
not sure, how the button works, but i assume it just opens the folder?
i would then go for sth. like the following following message:
To add stickers,
tap "Open Sticker Folder"
and drag image and sticker files to that folder.
to have less text, the error "No stickers yet" can also be removed, that seems obvious.
instead, the instructions can have a little larger font size
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.
addressed via 3ac757a
className='delta-button-round' | ||
onClick={onOpenStickerFolder} | ||
> | ||
{tx('open_sticker_folder')} |
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 button is too bold if there is already content.
the look of a secondary button is totally sufficient here (otherwise it looks as if it needs to be clicked to proceed)
(secondary button = transparent background, border in button color, if at all)
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.
addressed via 11443b4
lmk if you think the border should also be removed? currently just uses the existing style definition for the secondary button
Are stickers that are just in the sticker folder recognised or do they need to be into a "sticker pack" folder? If not we can either change that in core, adjust the text to say that, or create and open a default "sticker pack" folder. |
Closes #3258
Notes:
Preview:
Non-ideal state
Populated state
Interaction with populated state (first mouse, then keyboard):
Mouse:
Keyboard (not sure why the GIF tool I used decided to make this grainy 😅 ):