-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New room list: filter list can be collapsed #29992
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
d1587ee
to
21b542e
Compare
21b542e
to
f389547
Compare
f389547
to
15a4637
Compare
74c4fad
to
b4ec7c9
Compare
534aba2
to
d045fd9
Compare
d045fd9
to
39de042
Compare
39de042
to
f947866
Compare
f947866
to
f3674ce
Compare
padding: unset; | ||
list-style-type: none; | ||
/** | ||
* The InteractionObserver needs the height to be set for to work properly. |
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.
* The InteractionObserver needs the height to be set for to work properly. | |
* The InteractionObserver needs the height to be set to work properly. |
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.
* @param isExpanded - the filter is expanded or not (fully visible). | ||
* @param isVisible - `null` if there is not selected filter. `true` or `false` if the filter is visible or not. | ||
*/ | ||
function useFilters( |
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.
Why is this in the view and not the vm?
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 put it in the view because it depends of two view attributes: isExpanded
and isVisible
where isVisible
is the result of computation on the view
setFilterState({ | ||
filters: filters | ||
.slice() | ||
.sort((filterA, filterB) => (filterA.active === filterB.active ? 0 : filterA.active ? -1 : 1)), |
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.
Can we replace this chained ternary with an if-else?
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.
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.
The expand/collapse interaction seems rather moot for keyboard accessibility given tabbing accesses the hidden elements too, using tab you can also end up with the active filter off-screen and the room list looking somewhat confusing.
Screen.Recording.2025-05-29.at.09.52.08.mov
Maybe when accessing the list via keyboard the filters should expand automatically and the expand/collapse should not be part of the keyboard UX.
When tabbed to the last filter item the expand/collapse seems to disappear entirely. In this state, if I click off the element I am left in a broken state and cannot access the expand interaction.
Task https://github.com/element-hq/wat-internal/issues/204
Figma
This PR improves the primary filters of the new room list
Screen.Recording.2025-05-27.at.09.25.18.mov