-
-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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 navigation buttons below messages #31612
Conversation
87d6dcf
to
f3d9a75
Compare
402725f
to
3a70638
Compare
3a70638
to
471499c
Compare
I played around with this a bit and it seems to mostly work. I have the following notes:
Didn't note other obvious bugs playing around with it. |
471499c
to
08cc650
Compare
86f86a0
to
8dbab58
Compare
I think it works best not handle tab / shift + tab in the message view here too as we do for recent view and inbox.
Are we sure we want to remove the selection ring? |
8dbab58
to
803cf65
Compare
This is ready for another review. |
Yeah I'm not sure if either of those last two suggestions make sense. I found one new clear bug:
|
803cf65
to
2558f31
Compare
Fixed that. |
Test-deployed this on chat.zulip.org for discussion and feedback. |
Heads up @amanagr, we just merged some commits that conflict with the changes you made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
Heads up @amanagr, we just merged some commits that conflict with the changes you made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
Closing this since we are going in a slightly different direction and I don't want to lose this work in case we add something similar in the future. |
Fixes #30405
design discussion: CZO Link
Fix puppeteer tests
We want to add navigation buttons below the bottom message in the view. They should make it more convenient to perform common navigation actions, and remove the need for banners to mark messages as read.
End
, the "Scroll to bottom" button, and any other ways to jump to the bottom of the view should select this new navigation bar rather than the last message.All the bottom of message feed banners with a "Mark as read" button should be removed as part of this issue.
The initial goal is to get to a testable prototype, as we expect to need to iterate on the UX once we can play with it. It's fine to leave any tricky implementation details for later if they don't seem critical (with notes on the PR).
Which buttons, when and where
Home view
If
Esc
goes to home view:Mark as read
1st line:
Next unread conversation
This will resolve #18728.
DMs
P
Channel feed and topic views
N
Keyboard navigation behavior
Initially, the user will generally navigate to this bar as a whole (selected with the blue box). From there:
- In a conversation view, pressing Enter should start a reply to the current conversation, as it would on a message.
In non-conversation views, we can prototype with it doing nothing.
- Pressing Tab should select a button in the navigation bar.
- Pressing right arrow should select a button in the navigation bar.
We should probably remove the selection ring on the row as a whole at that point, but we'll have to test.
[ ]
- If "Mark as read" button is there, select that one.
- Otherwise, pick between "Home view" and "Next unread conversation"
depending on which one was used more recently, if both are present.
- Once one of the buttons is selected, right/left arrows and Tab/Shift+Tab should work to move between them, without cycling. (Maybe left from the leftmost and/or right from the rightmost would select the whole row again?)
Questions and follow-ups