MM-42482: [DRAFT, EXPECTING GUIDANCE] - Quick option to quote selected text

[sinansonmez]

Summary

This PR adds a new functionality to quickly quote the selected text

Ticket Link

Fixes https://github.com/mattermost/mattermost-server/issues/21539 JIRA: https://mattermost.atlassian.net/browse/MM-42482

Screenshots

To be added when the work is finalized

Release Note

New feature to quote selected text

[mattermod]

Hello @sinansonmez,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

[mattermod]

E2E tests not automatically triggered, because PR has no approval yet. Please ask a developer to review and then try again to attach the QA label.

[sinansonmez]

@esethna this is the first and very, very rough idea of my implementation. Please be aware that the code/ UI is not near to the end work.

I just opened the PR to check if my direction is correct without going further on the wrong way. Please let me know what devs think about it.

The idea here is to

1. Add eventListener to AdvancedCreatePost to get (i) selected text and (ii) cursor position
2. On the cursor position, show a button to basically get the selected text and paste into text box
3. After clicking, update this.state.message to include the selected text into text box

I assume, I need to do the same thing to AdvancedCreateComment to make it work when replying to message on RHS.

[mm-cloud-bot]

Creating a new SpinWick test server using Mattermost Cloud.

[mm-cloud-bot]

Mattermost test server created! :tada:

Access here: https://mattermost-webapp-pr-11480.test.mattermost.cloud

Account Type	Username	Password
Admin	sysadmin	Sys@dmin123
User	user-1	User-1@123

[M-ZubairAhmed]

removing myself as @michelengelen and @AshishDhama have worked on this component recently

[esethna]

@esethna this is the first and very, very rough idea of my implementation. Please be aware that the code/ UI is not near to the end work.

I just opened the PR to check if my direction is correct without going further on the wrong way. Please let me know what devs think about it.

The idea here is to

1. Add eventListener to AdvancedCreatePost to get (i) selected text and (ii) cursor position
2. On the cursor position, show a button to basically get the selected text and paste into text box
3. After clicking, update this.state.message to include the selected text into text box

I assume, I need to do the same thing to AdvancedCreateComment to make it work when replying to message on RHS.

@AshishDhama adding you here for reviewing this draft and providing some advice on direction as per @sinansonmez questions above ^

[esethna]

@sinansonmez thanks for the draft!

One thing that wasn't specified in the ticket is limitations. For example, I think we should only show this quick option when text is selected within post text (not other places in the app)

[esethna]

@Rishselva adding you to take over PM review and helping to sheppard this PR in. If you're open to it, please help make sure Sinan gets the help and guidance he needs to bring this to completion.

[sinansonmez]

@sinansonmez thanks for the draft!

One thing that wasn't specified in the ticket is limitations. For example, I think we should only show this quick option when text is selected within post text (not other places in the app)

@esethna exactly. This is one of the things in my mind that I should check. For instance, currently, you can select anything and it will be put into text editor which we dont want at all. That is why I marked this as draft. I will look into your point in the future iterations but I wanted to first confirm my direction.

[michelengelen]

Hi @sinansonmez ... Thanks for working on this.

all in all I think the general direction is good with. Just a few suggestions:

1. when inserting the text with handlePostQuote we should use the markdown handlers we use in the ATE text formatting as well (applyMarkdown function)
2. IMO we dont need to hide the button when scrolling out of 2 reasons: (i) the user might not expect it to disappear (or he accidentally scrolls) and depending on his intention this could lead to a bad experience since selecting the right text can be a hassle and (ii) every scroll handling is typically pretty unperformant, especially when tied to state changes
3. you are immediately calling toString() on the selection you get, but this is exactly where you could limit the places where it is allowed to select from, since it provides you with a ton of data about the selection (such as nodes and ranges, etc.)

Code-wise I will not go into much detail since this is an early draft/POC

One question for UX: @matthewbirtch have we ever thought about removing the standard contextmenu with our own implementation? Reason I am asking is: Showing the Quote button anytime I select something from a post would be disturbing IMO. We could build our own contextmenu and add several options to it we can control very precisely. WDYT?

[sinansonmez]

@michelengelen thanks for the feedback. I will try to move on this direction. I will ask your support in case of any issue.

You are right, showing a quote button every time a text is selected could be annoying.

For Quote button, I was thinking instead of showing a seperate Quote button, does make sense to add this option to dot (...) menu, similar to GitHub experience. Probably it is a question for @Rishselva @matthewbirtch

@michelengelen From technical perspective, is it harder to implement my suggested solution compared to current solution?

[matthewbirtch]

For Quote button, I was thinking instead of showing a seperate Quote button, does make sense to add this option to dot (...) menu, similar to GitHub experience. Probably it is a question for @Rishselva @matthewbirtch

I suppose we could, but this doesn't really save the user time or clicks then. It would be extra clicks to first select text, click the ••• button, then choose "Quote". With the quote button that is contextual to the selected text, all you do is select and click the quote button.

have we ever thought about removing the standard contextmenu with our own implementation? Reason I am asking is: Showing the Quote button anytime I select something from a post would be disturbing IMO. We could build our own contextmenu and add several options to it we can control very precisely. WDYT?

@michelengelen we're not going to go down this route right now since there are a lot of conflicting opinions about overriding the browser's context menu.

I don't think it will be that annoying to show the quote button when text from within a message is selected. As long as we only show this button when message text content is selected - not for any text element in the UI.

[esethna]

+1 to Matt's thoughts ^

[sinansonmez]

Thanks @matthewbirtch @esethna for the feedback. Then I will continue in the current direction taking into account Michel's comments

[esethna]

Adding @Rishselva to help usher this in. @sinansonmez feel free to reach out to her if you're blocked at all. Looking forward to this one!

[mattermod]

This PR has been automatically labelled "stale" because it hasn't had recent activity. A core team member will check in on the status of the PR to help with questions. Thank you for your contribution!

[Rishselva]

Hello @sinansonmez, any updates on this ticket? There hasn't been any recent activity but please let me know if there's anything I can help clarify :)

[sinansonmez]

Thanks for checking @Rishselva lately, I was busy with other PRs. As soon as I close 1-2 of them, i will keep working on this one.

[sinansonmez]

/update-branch

[sinansonmez]

/update-branch

[mattermod]

Error trying to update the PR. Please do it manually.

[sinansonmez]

/update-branch

[mattermod]

Error trying to update the PR. Please do it manually.

[sinansonmez]

/update-branch

[mattermod]

This PR has been automatically labelled "stale" because it hasn't had recent activity. A core team member will check in on the status of the PR to help with questions. Thank you for your contribution!

[sinansonmez]

@Rishselva I had a good progress here.

I implemented a new quote button, and it is working as expected in the main channel view (location of the button and rhs implementation is ongoing). Please give it a try

In order to be efficient, for now, I implemented an MVP which is quoting only works in pure text messages. This means that if selection includes mention, links, code, heading, quote or bullet points, it doesnt work. I will try to address them but before start working, I wanted to check with you the scope of the ticket.

From the following message format options, is there anything we can exclude:

• bold
• italic
• strike
• heading
• link
• list -ul or ol
• block quote
• code (could be excluded?)
• table (could be excluded?)

[esethna]

@sinansonmez we can probably exclude tables, but all others would be useful to be able to quote from.

[sinansonmez]

/update-branch

[esethna]

@sinansonmez thanks for your work on this, please let Matt and myself know when you're ready for review here!

[sinansonmez]

@esethna Thanks, it takes more time than I anticipated. Especially part related to positioning of the quote button for different type of posts (bold, quote, code etc,). I managed to finish good chunk of it. I will let you know when it is reqdy for review

[mm-cloud-bot]

New commit detected. SpinWick will upgrade if the updated docker image is available.