-
Notifications
You must be signed in to change notification settings - Fork 90
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 button to remove call from recents #2618
Conversation
Fixes: element-hq#2243 Signed-off-by: Johannes Marbach <[email protected]>
cool -- really nice. @amshakal wdyt ? |
👋🙂
This looks like the close icon from compound. Just pushed a change to use that instead of the trash can. How does that look? |
Perfect 👌👌👌 |
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 addition.
This is almost ready. Once the useTranslation
change is done I can approve it.
The other change is optional but I would be really happy and thankful if invest the time because you also think its an improvement.
This adds a button on each recent call tile to allow removing it.
Fixes: #2243
The trash can
IconButton
from compound is the least obtrusive option I could think of. It could be made even less obtrusive by only displaying it on hover but I wasn't sure if that's really needed.I didn't set the
destructive
property because while semantically true, it makes the button very prominent, especially when you have several tiles on the screen. Maybedestructive={true}
could be combined with only showing the button on hover though.One small annoyance is that because the
IconButton
is inside theLink
, the browser will show the link target when hovering the button. This felt better than reducing the tile's click target though.