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

FEATURE: chat redesign - back button to exit threads #24189

Merged
merged 9 commits into from Nov 7, 2023

Conversation

dbattersby
Copy link
Contributor

@dbattersby dbattersby commented Nov 1, 2023

This is part of the chat redesign work to improve chat navigation.

This change includes:

  • New header title with channel name (thread list on mobile)
  • New header title without channel name (thread list on full page chat)
  • Removes the close button on threads (mobile only)
  • Updates to back button route within thread (mobile), taking user to:
    • The thread index, if they accessed the thread from the thread index.
    • The channel itself, if they accessed the thread directly from the channel.
    • The channel itself, if they accessed the thread from a notification.
  • Show thread title in chat drawer header
  • Properly convert emoji in thread titles in chat header (all devices)
  • Upgrades various templates to use gjs format.

/t/-/112524

@github-actions github-actions bot added the chat PRs which include a change to Chat plugin label Nov 1, 2023
@dbattersby dbattersby marked this pull request as ready for review November 3, 2023 05:19
{{icon @icon}}
{{/if}}

{{this.headerTitle}}
Copy link
Contributor

Choose a reason for hiding this comment

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

generally I would prefer to avoid any text without any direct wrapper, here you have

div
icon span
text

  • text

I would recommend having two last text wrapped inside a span

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I've added wrapper for all of this info inside the chat drawer title so can easily style them or check existence in tests if needed.

@jjaffeux
Copy link
Contributor

jjaffeux commented Nov 3, 2023

@dbattersby I fixed various linting and js errors, I suspect your editor is misconfigured for gjs, let me know if you need help.

Other than this, It's nice if you can fix the few remaining comments, but already ok to merge like this.

@chancancode
Copy link
Contributor

👋🏼 just noticed this pull request! I opened #24260 today, but because the process is mostly automated, I could put that on hold and re-run it again after this has landed, if that makes things easier in. In the meantime, if you noticed anything that doesn't seem right in the conversation, feel free to let me know on there.

@jjaffeux
Copy link
Contributor

jjaffeux commented Nov 7, 2023

👋🏼 just noticed this pull request! I opened #24260 today, but because the process is mostly automated, I could put that on hold and re-run it again after this has landed, if that makes things easier in. In the meantime, if you noticed anything that doesn't seem right in the conversation, feel free to let me know on there.

Thanks, david warned me about the migration you are doing. We will merge this very soon.

@dbattersby dbattersby merged commit f20b6a0 into main Nov 7, 2023
14 checks passed
@dbattersby dbattersby deleted the chat-back-btn-to-exit-threads branch November 7, 2023 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chat PRs which include a change to Chat plugin
3 participants