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

compose: Show a first-time banner for jump to sent message conversation. #29590

Merged
merged 1 commit into from
Jun 5, 2024

Conversation

prakhar1144
Copy link
Member

@prakhar1144 prakhar1144 commented Apr 3, 2024

The first commit is from #29211 pulled here for testing the flow.

Show a one-time banner when jump to sent message conversation takes place for the first time.

Fixes: #29575

Screenshots and screen captures:

Screenshot from 2024-04-03 16-46-23

Self-review checklist
  • Self-reviewed the changes for clarity and maintainability
    (variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

  • Explains differences from previous plans (e.g., issue description).
  • Highlights technical choices and bugs encountered.
  • Calls out remaining decisions and concerns.
  • Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

  • Each commit is a coherent idea.
  • Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

  • Visual appearance of the changes.
  • Responsiveness and internationalization.
  • Strings and tooltips.
  • End-to-end functionality of buttons, interactions and flows.
  • Corner cases, error conditions, and easily imagined bugs.

@zulipbot
Copy link
Member

zulipbot commented Apr 3, 2024

Hello @zulip/design, @zulip/server-onboarding members, this pull request was labeled with the "area: onboarding", "UI experiment" labels, so you may want to check it out!

@zulipbot zulipbot added size: L and removed size: XL labels Apr 3, 2024
@prakhar1144 prakhar1144 marked this pull request as ready for review April 3, 2024 12:25
@alya
Copy link
Contributor

alya commented Apr 3, 2024

Hm, when I try this from Inbox, it looks like another banner flashes by first. I'm having a hard time capturing it in a screen recording (it's super quick), but maybe you can check in the code whether or not I'm imagining it?

@prakhar1144
Copy link
Member Author

Hm, when I try this from Inbox, it looks like another banner flashes by first. I'm having a hard time capturing it in a screen recording (it's super quick), but maybe you can check in the code whether or not I'm imagining it?

@alya Attaching a video for quick reference of the issue and screenshots to show final behavior:

banner-follow.webm

These message_sent_banners were causing issue:

  1. This is no longer needed as sender will be narrowed to that conversation. Removed it.
    Screenshot from 2024-04-29 11-58-24

  2. "Now Following" - one time banner (When a user automatically follows the topic). The banner will be visible along with the banner we're adding in this PR.

    Banners

@alya
Copy link
Contributor

alya commented Apr 29, 2024

I left a comment in #design.

@alya
Copy link
Contributor

alya commented Apr 29, 2024

Works for me otherwise!

When there are two banners, they don't appear simultaneously, but I think that's OK.

@prakhar1144 prakhar1144 force-pushed the jump-to-conversation-banner branch from 240b594 to c7dee29 Compare May 1, 2024 10:02
@prakhar1144
Copy link
Member Author

Updates in the recent push:

Screenshot from 2024-05-01 15-28-44

For @timabbott :

The order in which PRs should be integrated ideally:

  1. compose_banner: Replace the close button with "Got it" button.  #29910
  2. compose: Jump to conversation where message was sent. #29211
  3. This PR

@prakhar1144 prakhar1144 force-pushed the jump-to-conversation-banner branch 2 times, most recently from 7e83392 to 79673a0 Compare May 3, 2024 08:23
We immediately navigate the user to the conversation they just
sent a message to if they are not already in the appropriate
conversation view.

This commit adds a first-time banner to explain the same.

Fixes zulip#29575.
@timabbott timabbott force-pushed the jump-to-conversation-banner branch from 9a5aab5 to 22f3aeb Compare June 5, 2024 16:31
@timabbott
Copy link
Sponsor Member

Merged, thanks @prakhar1144!

@timabbott timabbott merged commit 22f3aeb into zulip:main Jun 5, 2024
6 of 7 checks passed
@zulipbot zulipbot added size: M and removed size: XL labels Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a first-time banner for jumping to sent message conversation
4 participants