MM-35722: CRT, post click open thread #8342

koox00 · 2021-07-01T15:18:40Z

Summary

Clicking, anywhere on a post in the center channel while having CRT on should open the thread in RHS,
even for single messages.

The difficulty in this is that posts may contain other elements that
have onClick handlers, or even just links and buttons. In those cases we
should not open the RHS.
Added a isEligibleForClick utility function that tries to solve the above
problem.

Ticket Link

https://mattermost.atlassian.net/browse/MM-35722

Release Note

NONE

Clicking on a post while having CRT on should open the thread in RHS, even for single messages. The difficulty in this is that posts may contain other elements that have onClick handlers, or even just links and buttons. In those cases we don't open the RHS. Added a isEligibleForClick utility function that tries to solve the above problem.

matthewbirtch · 2021-07-06T16:21:25Z

@koox00 Just played with this and noticed that if I click on a system join message, the screen goes blank. We should probably disable this feature on leave/join messages

koox00 · 2021-07-06T16:21:54Z

@koox00 Just played with this and noticed that if I click on a system join message, the screen goes blank. We should probably disable this feature on leave/join messages

yes I just noticed too, will fix thx!

utils/utils.jsx

components/post_view/post/post.jsx

nevyangelova

LGTM

BenCookie95

LGTM!

matthewbirtch · 2021-07-07T15:03:00Z

@koox00 we should test this in our threading sync with more content in the channels so we can all see how it feels. It seems ok with channels that only have a few threads, but I wonder how it might feel with full channels

matthewbirtch

@koox00 after playing with this a few times I think it's working quite well. The only thing I'm wondering is:

Can we make sure links, link previews, permalink previews, attachments or other internal interactive elements do not open the RHS? Right now it looks like clicking anywhere on the link preview container opens the RHS and clicking on attachments also opens the RHS (as well as the gallery view)

koox00 · 2021-07-07T17:50:39Z

Right now it looks like clicking anywhere on the link preview container opens the RHS and clicking on attachments also opens the RHS (as well as the gallery view)

Oh, thanks @matthewbirtch...

Can we make sure links, link previews, permalink previews, attachments or other internal interactive elements do not open the RHS?

That was my concern as well, we cannot be 100% sure something won't "break" it in the future.
I'll fix try to fix all those we use today, but when we add something new to the post view this functionality will be "broken", which normally would be OK since you could catch it but with this feature behind a feature flag it's a bit tricky.

calebroseland

LGTM! Difficult problem to solve "correctly."

Besides any pending fine tuning, my only thought might be to put a maintenance PSA comment somewhere, maybe in post.jsx?

~~// here be dragons~~

/**
 * @see {isEligibleForClick} — When adding clickable targets within a root post, please add to/maintain CLICKABLE_ELEMENTS
 */
 class Post ...

makeIsEligibleForClick takes as an argument a CSS selector string and returns a function to determine if the event's target is eligible to handle that particular onClick event. *The CSS selector argument matches elements that should not handle the event. Also standard HTMLElements that is very possible to have on click events on their own should not handle the event. See CLICKABLE_ELEMENTS.

matthewbirtch · 2021-07-08T14:41:57Z

@koox00 A couple things we noticed from the team testing session:

For the 'Update your status' tag that shows next to your username if you haven't ever used the custom status, clicking this opens up the RHS and shouldn't
For polls, the cursor shows the whole thing as clickable, but only the poll buttons are actually clickable
For link previews, the cursor shows the whole thing as clickable, but only the link title is actually clickable. This may be another PR, but just like the proposed permalink previews work, maybe the whole link preview container should actually be clickable

BenCookie95 · 2021-07-08T15:51:11Z

utils/utils.jsx

+ * @param {string} selector - CSS selector of elements not eligible for click
+ * @returns {function}
+ */
+export function makeIsEligibleForClick(selector) {


Can we add a default of empty for this?

jgilliam17

Thanks @koox00
Tested, looks good to merge.

Verified click on a post opens RHS thread.
Issue with dismissing profile popover has been filed separately. https://mattermost.atlassian.net/browse/MM-37291

mm-cloud-bot · 2021-07-20T18:39:53Z

Test server destroyed

jgilliam17 · 2021-07-20T18:40:30Z

/update-branch

esethna · 2021-07-20T19:40:29Z

@jgilliam17 if we don't have one can we file a ticket for the issues we discussed in planning where this takes effect even if menus/modals etc are open

jgilliam17 · 2021-07-20T19:45:51Z

Can we repurpose the ticket I created for the profile popover? https://mattermost.atlassian.net/browse/MM-37291

jgilliam17 · 2021-07-20T19:46:41Z

@esethna see above

esethna · 2021-07-20T19:48:29Z

@jgilliam17 sure let's just add details as needed for other cases

amyblais · 2021-07-20T22:34:18Z

/cherry-pick cloud

mattermod · 2021-07-20T22:34:19Z

Cherry pick is scheduled.

@returns

* Naive post click open thread Clicking on a post while having CRT on should open the thread in RHS, even for single messages. The difficulty in this is that posts may contain other elements that have onClick handlers, or even just links and buttons. In those cases we don't open the RHS. Added a isEligibleForClick utility function that tries to solve the above problem. * Fixes: clicking on system messages * Minor change * Refactors isEligibleForClick to makeIsEligibleForClick makeIsEligibleForClick takes as an argument a CSS selector string and returns a function to determine if the event's target is eligible to handle that particular onClick event. *The CSS selector argument matches elements that should not handle the event. Also standard HTMLElements that is very possible to have on click events on their own should not handle the event. See CLICKABLE_ELEMENTS. * Fixes JSDoc @returns * Fixes tests * Fixes tests * Minor styling fixes Attachments should set cursor to default. Update your status button should be a button. makeIsEligibleForClick initial value for argument selector. * Fixes tests Co-authored-by: Mattermod <mattermod@users.noreply.github.com> (cherry picked from commit 6ed8a49)

@returns

* Naive post click open thread Clicking on a post while having CRT on should open the thread in RHS, even for single messages. The difficulty in this is that posts may contain other elements that have onClick handlers, or even just links and buttons. In those cases we don't open the RHS. Added a isEligibleForClick utility function that tries to solve the above problem. * Fixes: clicking on system messages * Minor change * Refactors isEligibleForClick to makeIsEligibleForClick makeIsEligibleForClick takes as an argument a CSS selector string and returns a function to determine if the event's target is eligible to handle that particular onClick event. *The CSS selector argument matches elements that should not handle the event. Also standard HTMLElements that is very possible to have on click events on their own should not handle the event. See CLICKABLE_ELEMENTS. * Fixes JSDoc @returns * Fixes tests * Fixes tests * Minor styling fixes Attachments should set cursor to default. Update your status button should be a button. makeIsEligibleForClick initial value for argument selector. * Fixes tests Co-authored-by: Mattermod <mattermod@users.noreply.github.com> (cherry picked from commit 6ed8a49) Co-authored-by: Kyriakos Z <3829551+koox00@users.noreply.github.com>

Documentation for: mattermost/mattermost-webapp#8342 Updated: - Messages > Work with Messages > Organizing Conversations using Collapsed Reply Threads (Beta) > Start or Reply to Threads - Updated Tip bullet point #1 to clarify that uses can click anywhere in a message in the center pane to view it/reply to it

* Update conf.py * Update open-source-components.rst * Update release-lifecycle.rst * Reliable Websockets: default to true Documentation for: mattermost/mattermost#17890 Updated: - Set Up, Manage, Onboard, and Comply > Set Up Mattermost > Self-Managed Deployments > Configuration Settings > Experimental Settings only in config.json > Enable Reliable Websockets - Updated the default value of the config setting to ``true`` * PR didn't follow internal processes for submission * Re-added file deleted in error * Re-added the correct file that was deleted in error * MM-36779 - Add ChimeraOAuthProxyURL config option (#4928) * MM-36779 - Add ChimeraOAuthProxyURL config option Documentation for: mattermost/mattermost#17888 Updated: - Set Up, Manage, Onboard, and Comply > Set Up Mattermost > Self-Managed Deployments > Configuration Settings > Plugins (Beta) - Added the Chimera OAuth Proxy URL configuration setting * Added ChimeraOAuthProxyUrl to Telemetry * added note about the setting being available only via config.json * Update source/configure/configuration-settings.rst Co-authored-by: Justine Geffen <justinegeffen@users.noreply.github.com> * Update source/configure/configuration-settings.rst Co-authored-by: Justine Geffen <justinegeffen@users.noreply.github.com> Co-authored-by: Justine Geffen <justinegeffen@users.noreply.github.com> * Reliable Websockets: default to true (#4918) Documentation for: mattermost/mattermost#17890 Updated: - Set Up, Manage, Onboard, and Comply > Set Up Mattermost > Self-Managed Deployments > Configuration Settings > Experimental Settings only in config.json > Enable Reliable Websockets - Updated the default value of the config setting to ``true`` * Added functionality to archive/unarchive teams from system console (#4927) * Added functionality to archive/unarchive teams from system console Documentation for: mattermost/mattermost-webapp#8129 Updated: - Set Up, Manage, Onboard, and Comply > Manage Mattermost > Self-Managed Deployments > Managing Team and Channel Members (E20) > Team Profile - Updated section to include details on how to archive/unarchive the team * Added archive/unarchive updates to Cloud-specific page * Updated LHS - Moved self-managed topic from all instances to self-managed - Added Cloud-specific topic * added mmctl equivalent for team restore * added mmctl equivalent for team restore (self-managed) * Update source/manage/cloud-team-and-channel.rst Co-authored-by: Justine Geffen <justinegeffen@users.noreply.github.com> * Update source/manage/team-channel-members.rst Co-authored-by: Justine Geffen <justinegeffen@users.noreply.github.com> * Update data-retention-policy.rst (#4937) * Update data-retention-policy.rst Adding docs for custom data retention policies. * Update data-retention-policy.rst * Update source/comply/data-retention-policy.rst Co-authored-by: Carrie Warner (Mattermost) <74422101+cwarnermm@users.noreply.github.com> * Update source/comply/data-retention-policy.rst Co-authored-by: Carrie Warner (Mattermost) <74422101+cwarnermm@users.noreply.github.com> * Update source/comply/data-retention-policy.rst Co-authored-by: Carrie Warner (Mattermost) <74422101+cwarnermm@users.noreply.github.com> * Update source/comply/data-retention-policy.rst Co-authored-by: Carrie Warner (Mattermost) <74422101+cwarnermm@users.noreply.github.com> * Update source/comply/data-retention-policy.rst Co-authored-by: Carrie Warner (Mattermost) <74422101+cwarnermm@users.noreply.github.com> * Update source/comply/data-retention-policy.rst Co-authored-by: Carrie Warner (Mattermost) <74422101+cwarnermm@users.noreply.github.com> * Update source/comply/data-retention-policy.rst Co-authored-by: Carrie Warner (Mattermost) <74422101+cwarnermm@users.noreply.github.com> * Update source/comply/data-retention-policy.rst Co-authored-by: Carrie Warner (Mattermost) <74422101+cwarnermm@users.noreply.github.com> * Update source/comply/data-retention-policy.rst Co-authored-by: Carrie Warner (Mattermost) <74422101+cwarnermm@users.noreply.github.com> * Update source/comply/data-retention-policy.rst Co-authored-by: Carrie Warner (Mattermost) <74422101+cwarnermm@users.noreply.github.com> * Update source/comply/data-retention-policy.rst Co-authored-by: Carrie Warner (Mattermost) <74422101+cwarnermm@users.noreply.github.com> * Update source/comply/data-retention-policy.rst Co-authored-by: Carrie Warner (Mattermost) <74422101+cwarnermm@users.noreply.github.com> * Update source/comply/data-retention-policy.rst Co-authored-by: Carrie Warner (Mattermost) <74422101+cwarnermm@users.noreply.github.com> * Update data-retention-policy.rst * Update data-retention-policy.rst * Update source/comply/data-retention-policy.rst Co-authored-by: Carrie Warner (Mattermost) <74422101+cwarnermm@users.noreply.github.com> * Update source/comply/data-retention-policy.rst Co-authored-by: Carrie Warner (Mattermost) <74422101+cwarnermm@users.noreply.github.com> * Update source/comply/data-retention-policy.rst Co-authored-by: Carrie Warner (Mattermost) <74422101+cwarnermm@users.noreply.github.com> * Update source/comply/data-retention-policy.rst Co-authored-by: Carrie Warner (Mattermost) <74422101+cwarnermm@users.noreply.github.com> Co-authored-by: Carrie Warner (Mattermost) <74422101+cwarnermm@users.noreply.github.com> * Update important-upgrade-notes.rst (#4811) * Update important-upgrade-notes.rst * Update important-upgrade-notes.rst * Update important-upgrade-notes.rst * Fix typos * MM-36547 Renamed 'Mute Channel' and 'Edit Channel Header' menus (#4941) Documentation for: mattermost/mattermost-webapp#8313 Updated: - Messaging > Manage Teams, Channels, and Members > Managing Channels > Creating a Direct or Group Message - Updated section to include mute, unmute, and edit header options for group and direct messages * MM-35722: CRT, post click open thread (#4942) Documentation for: mattermost/mattermost-webapp#8342 Updated: - Messages > Work with Messages > Organizing Conversations using Collapsed Reply Threads (Beta) > Start or Reply to Threads - Updated Tip bullet point #1 to clarify that uses can click anywhere in a message in the center pane to view it/reply to it * MM-35470 Disable config watching logic (#4951) * MM-35470 Disable config watching logic Documentation for: mattermost/mattermost#17913 Updated: - Set Up, Manage, Onboard, and Comply > Set Up Mattermost > Self-Managed Deployments > Configuration Settings - Updated introductory content to remove < v5.12 legacy note and add config watcher note and link to mmctl command * Update source/configure/configuration-settings.rst Co-authored-by: Justine Geffen <justinegeffen@users.noreply.github.com> * v5.38.0 Changelog (#4810) * Update self-managed-changelog.md * Update self-managed-changelog.md * Update self-managed-changelog.md * Update self-managed-changelog.md * Update self-managed-changelog.md * Update self-managed-changelog.md * Update self-managed-changelog.md * Apply suggestions from code review Co-authored-by: Carrie Warner (Mattermost) <74422101+cwarnermm@users.noreply.github.com> * Update self-managed-changelog.md * Update self-managed-changelog.md * Update self-managed-changelog.md Co-authored-by: Carrie Warner (Mattermost) <74422101+cwarnermm@users.noreply.github.com> Co-authored-by: Amy Blais <29708087+amyblais@users.noreply.github.com> Co-authored-by: Justine Geffen <justinegeffen@users.noreply.github.com> Co-authored-by: Katie Wiersgalla <39744472+wiersgallak@users.noreply.github.com>

koox00 requested review from hmhealey, calebroseland, BenCookie95 and nevyangelova July 1, 2021 15:18

mm-cloud-bot added the release-note-none label Jul 1, 2021

koox00 added the 2: Dev Review Requires review by a core commiter label Jul 1, 2021

koox00 requested a review from matthewbirtch July 6, 2021 16:04

matthewbirtch added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Jul 6, 2021

koox00 added 2 commits July 6, 2021 19:31

Fixes: clicking on system messages

9ca54b3

Minor change

8cc25b4

hmhealey reviewed Jul 6, 2021

View reviewed changes

utils/utils.jsx Outdated Show resolved Hide resolved

hmhealey approved these changes Jul 6, 2021

View reviewed changes

koox00 mentioned this pull request Jul 7, 2021

[MM-36175] Permalink previews #8304

Merged

nevyangelova reviewed Jul 7, 2021

View reviewed changes

components/post_view/post/post.jsx Show resolved Hide resolved

nevyangelova approved these changes Jul 7, 2021

View reviewed changes

BenCookie95 approved these changes Jul 7, 2021

View reviewed changes

matthewbirtch suggested changes Jul 7, 2021

View reviewed changes

calebroseland approved these changes Jul 7, 2021

View reviewed changes

koox00 added 3 commits July 8, 2021 15:44

Fixes JSDoc @returns

481d350

Fixes tests

2b46822

koox00 requested a review from matthewbirtch July 8, 2021 13:31

Fixes tests

e9379c4

BenCookie95 reviewed Jul 8, 2021

View reviewed changes

jgilliam17 approved these changes Jul 20, 2021

View reviewed changes

jgilliam17 added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester Setup Cloud Test Server Setup a test server using Mattermost Cloud labels Jul 20, 2021

Merge branch 'master' into MM-35722_crt-click-thread-center

eaf2ab5

amyblais assigned koox00 Jul 20, 2021

jgilliam17 merged commit 6ed8a49 into mattermost:master Jul 20, 2021

amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Jul 20, 2021

mattermost-build mentioned this pull request Jul 20, 2021

Automated cherry pick of #8342 #8436

Merged

mattermod added the CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone label Jul 20, 2021

esethna added Docs/Needed Requires documentation and removed Docs/Not Needed Does not require documentation labels Jul 21, 2021

cwarnermm mentioned this pull request Aug 9, 2021

MM-35722: CRT, post click open thread mattermost/docs#4942

Merged

amyblais added Docs/Done Required documentation has been written and removed Docs/Needed Requires documentation labels Aug 11, 2021

mattermod mentioned this pull request Oct 6, 2021

CRT: Prevent 'opening thread on post click' when dismissing a popover mattermost/mattermost#18575

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MM-35722: CRT, post click open thread #8342

MM-35722: CRT, post click open thread #8342

koox00 commented Jul 1, 2021 •

edited

Loading

matthewbirtch commented Jul 6, 2021

koox00 commented Jul 6, 2021

nevyangelova left a comment

BenCookie95 left a comment

matthewbirtch commented Jul 7, 2021

matthewbirtch left a comment •

edited

Loading

koox00 commented Jul 7, 2021 •

edited

Loading

calebroseland left a comment •

edited

Loading

matthewbirtch commented Jul 8, 2021

BenCookie95 Jul 8, 2021

jgilliam17 left a comment

mm-cloud-bot commented Jul 20, 2021

jgilliam17 commented Jul 20, 2021

esethna commented Jul 20, 2021

jgilliam17 commented Jul 20, 2021

jgilliam17 commented Jul 20, 2021

esethna commented Jul 20, 2021

amyblais commented Jul 20, 2021

mattermod commented Jul 20, 2021

MM-35722: CRT, post click open thread #8342

MM-35722: CRT, post click open thread #8342

Conversation

koox00 commented Jul 1, 2021 • edited Loading

Summary

Ticket Link

Release Note

matthewbirtch commented Jul 6, 2021

koox00 commented Jul 6, 2021

nevyangelova left a comment

Choose a reason for hiding this comment

BenCookie95 left a comment

Choose a reason for hiding this comment

matthewbirtch commented Jul 7, 2021

matthewbirtch left a comment • edited Loading

Choose a reason for hiding this comment

koox00 commented Jul 7, 2021 • edited Loading

calebroseland left a comment • edited Loading

Choose a reason for hiding this comment

matthewbirtch commented Jul 8, 2021

BenCookie95 Jul 8, 2021

Choose a reason for hiding this comment

jgilliam17 left a comment

Choose a reason for hiding this comment

mm-cloud-bot commented Jul 20, 2021

jgilliam17 commented Jul 20, 2021

esethna commented Jul 20, 2021

jgilliam17 commented Jul 20, 2021

jgilliam17 commented Jul 20, 2021

esethna commented Jul 20, 2021

amyblais commented Jul 20, 2021

mattermod commented Jul 20, 2021

koox00 commented Jul 1, 2021 •

edited

Loading

matthewbirtch left a comment •

edited

Loading

koox00 commented Jul 7, 2021 •

edited

Loading

calebroseland left a comment •

edited

Loading