-
-
Notifications
You must be signed in to change notification settings - Fork 350
feat(ui): Add text labels to action buttons for clarity #813
base: main
Are you sure you want to change the base?
feat(ui): Add text labels to action buttons for clarity #813
Conversation
Thanks for your contribution :) I would suggest to use tooltips instead of adding the text directly to the buttons. This would keep the UI compact but still describe each action for new users. |
Hi @stonith404, thank you for the feedback! That's a good point about keeping the UI compact. My initial reason for adding text directly was for mobile users, since tooltips aren't really a thing there. But if you think tooltips are the better overall approach, I'm happy to switch to that |
I just think especially on mobile the buttons just take too much space: But regarding mobile users, you could use the Mantine Tooltip which also supports touch devices. |
@stonith404 I hadn't realized Mantine Tooltip was also available on mobile. Since that's the case, I'll adjust the PR and switch to using tooltips! |
@stonith404 Please take a look over, when you get a chance! |
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.
Sorry for my late response. Here are some things I would recommend to change.
<ActionIcon> | ||
<Avatar size={28} /> | ||
</ActionIcon> | ||
<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.
@@ -39,7 +39,8 @@ const DownloadAllButton = ({ shareId }: { shareId: string }) => { | |||
|
|||
return ( | |||
<Button | |||
variant="outline" | |||
variant="light" |
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.
<ActionIcon color="orange" variant="light" size={25}> | ||
<TbEdit /> | ||
</ActionIcon> | ||
<Tooltip |
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.
Could you create a component that wraps Tooltip
with its common properties? For example the position
, multiline
and events
are the same for every tooltip.
Description:
This PR enhances UI clarity by adding descriptive text labels to header icons and implementing tooltips for the main action icon buttons.
Motivation:
The goal is to improve usability and reduce potential confusion for all users. Explicit text labels in the header section make the function of those elements immediately clear. For the main action buttons (like Info, Undo, etc.), tooltips were added to clarify icon function upon hover, providing necessary context without permanently adding text to that potentially dense button area.
Changes:
npm run format
.Visuals:
Before:
After: