Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Spotlight: don't join a public room when clicking on it #12579

Closed
wants to merge 12 commits into from

Conversation

florianduros
Copy link
Contributor

@florianduros florianduros commented Jun 5, 2024

Checklist

  • Tests written for new code (and old code if feasible).
  • New or updated public/exported symbols have accurate TSDoc documentation.
  • Linter and other CI checks pass.
  • Sign-off given on the changes (see CONTRIBUTING.md).

Closes element-hq/element-web#27523

For public room, the listener is only reacting to keyboard event for keyboard/accessibility navigation. The list element already has a button to join or preview a room for the mouse user.

@florianduros florianduros added the T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems label Jun 5, 2024
Copy link
Member

@t3chguy t3chguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has a very bad UX right now. The hover effect + cursor pointer makes it look clickable but clicking on it does nothing, it feels broken.

@t3chguy
Copy link
Member

t3chguy commented Jun 7, 2024

Given this moves away from the designs provided to us I think we will want a design review to confirm they are happy with the new UX.

@t3chguy t3chguy requested a review from a team June 7, 2024 08:46
Copy link

@gaelledel gaelledel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx - all good. To think about if we're willing to change the colour on Hover now or later.

@florianduros
Copy link
Contributor Author

florianduros commented Jul 4, 2024

@gaelledel the colour change should be easy, I will do in the PR

Copy link
Member

@t3chguy t3chguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keyboard accessibility seems broken. Enter is not working to activate the tile

Screen.Recording.2024-07-05.at.08.15.51.mov

Otherwise this does what it says on the tin but I am worried about the inconsistency in the Spotlight dialog when you switch into Public rooms/spaces more. Prior the whole tile is clickable, after only the button is clickable. I feel like this inconsistency is not ideal for a11y.

@florianduros
Copy link
Contributor Author

@t3chguy Yes, there are some inconsistencies but the short term is to fix this issue. @gaelledel and I came with this small change in order to avoid massive ux changes of the spotlight.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No way to copy aliases/ids from the room directory without accidentally joining rooms
4 participants