-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Adds tooltip for compose menu #31122
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
base: develop
Are you sure you want to change the base?
Conversation
|
|
||
| const MenuTrigger = ({ ref, ...props }: MenuTriggerProps): JSX.Element => ( | ||
| <Tooltip label={_t("action|start_chat")}> | ||
| <IconButton aria-label={_t("action|add")} {...props} ref={ref}> |
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.
I believe the aria-label is overwritten by the label of the parent Tooltip, causing your tests to fail. I believe this uses Add because the menu also lets you create rooms so Start chat would be a misleading tooltip for that
Also there should probably be a tooltip consistently between the Start chat single-button variant of this button and the menu-trigger "Add" variant
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.
I chose 'Start chat' because 'Add' did not seem clear to me. ("Add what?") I assumed that a room was also a chat (group chat). I see that this is misleading in terms of the element naming convention. Especially if the button also allows to create a video room. Would 'Add room' fit the element naming convention? A chat is also a room. I guess our users could understand that. Otherwise, I will try to come up with a better tooltip or just use 'Add'.
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.
Would 'Add room' fit the element naming convention?
It'd fit within the Matrix naming convention but I think Design would not be happy as they definitely do not want DMs to be seen as rooms, so that would definitely need design sign-off which can take a while to get.
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.
New conversation? "Conversation" is alrady being used in some places for room and dms. e.g:
- "Start a conversation with someone using their name or username (like <userId/>)."
- "Recent Conversations"
- "Own your conversations."
src/components/views/rooms/RoomListPanel/RoomListHeaderView.tsx
Outdated
Show resolved
Hide resolved
df91fe4 to
c7a26ec
Compare
c7a26ec to
ba90f3a
Compare
|
@byteplow the PR has lint errors, you can run |
Adds tooltip
New conversationto Compose Menu Button In Room List Header.Checklist
public/exportedsymbols have accurate TSDoc documentation.