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.

Sign up for GitHub

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

Jump to bottom

compose: Quote part of a message (Fixes #19712) #21834

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

compose: Quote part of a message (Fixes #19712) #21834

wants to merge 5 commits into from

Conversation

asah
Copy link
Collaborator

@asah asah commented Apr 17, 2022

Fixes #19712

(this is PR replaces the original one that was accidentally deleted - same code)

Tested on FF, Chrome and Safari.

chrome-capture (1)

chrome-capture (2)

chrome-capture (3)

notes:

  • selection: tested with text, links, random text on the screen (eg. + invite users)
  • quote_and_reply trigger: tested with shortcut, message menu
  • browser: chrome, firefox, safari
  • note: on menu-click, Safari loses focus (blur) and updates the selection to the empty string. I can ignore empty string, but then other blur events (e.g. clicking +invite user) will retain the old selection, which becomes weird when you then reply to a message. We could have Safari only support partial-quoting when using the keyboard (>) but not the menu?

project checklist:

  • design review (alya)

  • eng design review ()

  • unittests (asah)

@zulipbot zulipbot added size: XL area: compose (misc) rust community request Issues of interest to the Rust community labels Apr 17, 2022
@asah
Copy link
Collaborator Author

asah commented Apr 17, 2022

@alya next round of UI/UX ready for your review.

@asah asah marked this pull request as draft April 17, 2022 22:45
@alya
Copy link
Contributor

alya commented Apr 19, 2022

Hm, playing around with this, it seems like whenever I have the end of a message selected, the entire message ends up in the quote-and-reply (whether I use the mouse or the keyboard shortcut).

@zulipbot
Copy link
Member

Heads up @asah, 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.

@timabbott timabbott force-pushed the main branch 2 times, most recently from 4ec3636 to 88b200c Compare August 18, 2023 23:51
@timabbott timabbott added area: message feed (UI) Buttons/UI directly in the message feed (not popovers, etc.) area: compose (formatting) Compose box formatting UI (keyboard and mouse) and removed area: compose (misc) labels Sep 19, 2023
@timabbott timabbott self-assigned this Jan 15, 2024
@timabbott timabbott added the completion candidate PRs with reviews that may unblock merging label Jan 15, 2024
@timabbott timabbott mentioned this pull request Jan 15, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@timabbott timabbott

Labels
area: compose (formatting) Compose box formatting UI (keyboard and mouse) area: message feed (UI) Buttons/UI directly in the message feed (not popovers, etc.) completion candidate PRs with reviews that may unblock merging has conflicts rust community request Issues of interest to the Rust community size: XL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Quote part of a message
4 participants
@asah @alya @zulipbot @timabbott