Skip to content
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

views: Add footer to StreamsView and TopicsView #1199

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

plugyawn
Copy link
Collaborator

@plugyawn plugyawn commented Apr 14, 2022

What does this PR do?
Added the shortcut hint for displaying Topics to the Streams Panel and a hint to get back to Streams in the Topics Panel.

Associated with #1190. Discussed on CZO at Stream/topic toggle hint?

Tested?

  • Manually
  • Existing tests (adapted, if necessary)
  • New tests added (for any new behavior)
  • Passed linting & tests (each commit)

Commit flow

  • first commit adds the footer for the hints.
  • upcoming commit will add functionality for hint to only appear when cursor is in the panel.

Notes & Questions

  • Still shows hint even when not in the left panel, causing probable misunderstanding about where one has to press t, so have to fix that.

Visual changes
Screenshot 2022-04-15 at 4 28 32 AM

@zulipbot zulipbot added the size: M [Automatic label added by zulipbot] label Apr 14, 2022
@neiljp
Copy link
Collaborator

neiljp commented Apr 15, 2022

@plugyawn Thanks for exploring this 👍 See my thoughts in the stream, since this is mainly visual so we may want to discuss rather than focus on implementation.

@neiljp neiljp added the PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback label Apr 15, 2022
@plugyawn
Copy link
Collaborator Author

@neiljp implemented the theming change. Is it clearer to look at now?

@plugyawn
Copy link
Collaborator Author

@zulipbot add "PR needs review"

@zulipbot zulipbot added the PR needs review PR requires feedback to proceed label Apr 17, 2022
@neiljp neiljp removed the PR needs review PR requires feedback to proceed label Apr 17, 2022
@neiljp
Copy link
Collaborator

neiljp commented Apr 17, 2022

Feedback was given in the stream.

plugyawn added 2 commits May 6, 2022 19:06
Added the shortcut hint for displaying Topics to
the Streams Panel and a hint to get back to Streams
in the Topics Panel.
@plugyawn plugyawn force-pushed the issue-1190-show-topic-hint branch from 9771985 to 7958751 Compare May 6, 2022 13:38
@neiljp
Copy link
Collaborator

neiljp commented May 11, 2022

@plugyawn You didn't change the labels on this, though have pushed since we last reviewed. I left a note in the stream.

@plugyawn
Copy link
Collaborator Author

plugyawn commented May 12, 2022

@plugyawn You didn't change the labels on this, though have pushed since we last reviewed. I left a note in the stream.

Oh, I just fixed the commit text for the PR, didn't add anything new, so I didn't change the labels.

@plugyawn plugyawn added PR needs review PR requires feedback to proceed PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback and removed PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback PR needs review PR requires feedback to proceed labels May 12, 2022
@zulipbot
Copy link
Member

Heads up @plugyawn, 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 upstream/main branch and resolve your pull request's merge conflicts accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has conflicts PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback size: M [Automatic label added by zulipbot]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants