[MM-22622] Add mentionHighlightDisabled props on create post and create comment #4916

fmunshi · 2020-02-16T18:09:57Z

Summary

Adds a check in the markdown component to set options.mentionHighlight to false if props.mentionHighlightDisabled is true
Sets mentionHighlightDisabled to true when creating a comment or post
(this is also checked on the server but it is also needed on the webapp or else the @ channel text will temporarily flash as highlighted before the message gets validated on the server)
Tests

Ticket Link

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

Related Pull Requests

Has Server changes: [MM-22622] Add Ephemeral response when using mentions without permissions mattermost#13902

Screenshots

…ment without highlights

hmhealey · 2020-02-23T16:39:19Z

components/create_comment/create_comment.jsx

+ ...draft,
+ props: {
+ ...draft.props,
+ mentionHighlightDisabled: true,


Can we trust the client to set this correctly? What if a user that can't do channel-wide mentions were to use the API to manually send a post without this field? It feels to me like this is something that the server would set.

The server pr (mattermost/mattermost#13902) also ensures the props are set if the user is lacking permission to channel mention.

The reason I also have it here is for the split second immediately after creating the post but before the post is actually validated on the server. During that time the message would show up as highlighted (for the user sending the post) if the client did not also set the prop.

…side-markdown

srkgupta

Hi @fm2munsh

Found some issues. I am not sure if the fixes for all these issues will belong to webapp, but if not, please feel free to add a comment in the corresponding server repo and fix the same there.

A JS error is thrown in the console, when a user with channel admin permission uses the mention @all or @here. Note: in this case server does not sends the props attribute in the create post API response and may be the webapp code expects it and throws an error. Can you please check and fix this. This JS error is not shown when a regular member uses the channel mentions and gets an error that the mention did not trigger any notifications.
When a regular member who does not has use_channel_mentions permission updates the channel header and uses like @all or @here, as shown in the screenshot, the same mention gets highlighted in the system message.

Expected: Since user does not has any permissions to use channel mentions, these keywords @all, @here, @channel should not get highlighted even when the user tries to update the channel header or channel purpose.

The message Channel notifications are disabled.... is not displayed when a regular members edits a post.
a) As a regular member without use_channel_mentions permission, write a simple post testing
b) Now edit that post and use one of the channel mentions like testing @all
In this case, the message Channel notifications are disabled in the CHANNEL_NAME. The @all did not trigger any notifications is not displayed.

fmunshi · 2020-03-02T17:15:52Z

@srkgupta I dont think the first issue you highlighted is related to my PR since I get that same console error on community.mattermost.com and my local master branch

I think a new ticket for the console error may be more appropriate.

As for the other two issues (channel header and editing messages with mentions) I brought them up with @michaelgamble in a UX review and based on what he thinks the appropriate UX for it will be I will modify the behaviour for those two cases.

fmunshi · 2020-03-02T17:30:29Z

@srkgupta After looking into it some more that console error seems to be an error coming from the Lastpass chrome extension - if you disable lastpass on the page and then try sending a message you wont see the error

srkgupta · 2020-03-02T17:33:14Z

Thanks @fm2munsh. Yes indeed it was due to the LastPass Extension. The issue was actually not seen on my incognito window where the LastPass extension wasnt enabled. I disabled it on my main window as well and the error was no longer seen. Thanks for checking it.

fmunshi · 2020-03-03T01:41:50Z

@srkgupta I have resolved the second issue - The behaviour implemented is that the channel header will ignore @ all @ here @ channel in all cases and treat them as normal text regardless of user permission since it was never triggering notifications anyway.

As for the third issue of editing a post I discussed it with @michaelgamble and since currently notifications do not get triggered when editing a post we will open a new ticket to handle sending notifications when adding mentions to edited posts. (I.e. It will not be handled as part of this ticket and the behaviour we have right now is sufficient)

srkgupta · 2020-03-03T10:16:21Z

Thanks @fm2munsh I have raised that issue separately here:
https://mattermost.atlassian.net/browse/MM-22866

srkgupta

Tested the changes along with the server PR mattermost/mattermost#13902 and the feature is working fine as expected. Approving the PR. Tests have been added in the JIRA for this feature at the epic level.

sudheerDev

LGTM 👍

michaelgamble

Reviewed over zoom with @fm2munsh everything looks good to go.

…te comment (mattermost#4916) * MM-22622 Send mentionHighlighDisabled props when creating post or comment without highlights * MM-22622 Disable mention highlight on system header change message * MM-22622 Fix snapshot tests

@deanwhillier

…ript (#4975) * [MM-20546]Migrate 'components/user_settings/security/mfa_section' module and associated tests to TypeScript #13695 * Fix lint check * Fix lint check fail * Fix lint failed check * Fix type error for user.mfa_active * Update action type and test cases for mfa_section * Fix lint check failure * Fix CI test failure * [MM-22137] - Make RHS comment dropdown open down by default (#4901) * [MM-22137] - Make RHS comment dropdown open down by default * Recalculate openUp based on space on top and bottom of the trigger * resolve PR comments * [MM-22389] Refactor adminResetPassword action to adhere to redux thun… (#4891) * [MM-22389] Refactor adminResetPassword action to adhere to redux thunk standard * Please linter * Use updateUserPassword redux action instead * Remove redundant console.log * Fix type errors * Remove redundant action * Fix type errors Co-authored-by: mattermod <[email protected]> * Added Cypress test to verify Invite via Email containing upper case letters (#4823) * Added Cypress test to verify Invite via Email containing upper case letters * Added step to reset Guest Settings & to visit team page Co-authored-by: mattermod <[email protected]> * [MM-22204] - Add target="_blank" to terms and policy links (#4912) I propose we make a ticket to refactor the formatted_markdown_message.jsx because adding the '!' to activate target="_blank" is a very obscure way to do it and it was a undocumented feature. * [MM-22129] - Remove post link modal and copy permalink directly (#4930) * [MM-22129] - Remove post link modal and copy permalink directly * Remove redundant translations * Change Permalink text * Fix test * Add latest mattermost-redux version (#4947) * MM-22079 Properly capture emoji sequences (#4859) * MM-22079 Update tests to include more types of emojis * MM-22079 Properly capture emoji sequences * Fix types and styling * Fix test description * MM-22590 Fix messages (#4838) * Replace IE with Edge * Add translation string * Make messages translated * Fix the order of message * Fix default message * Update snapshot Co-authored-by: mattermod <[email protected]> * Update en.json (#4877) Automatic Merge * Fixes #9506 : Keyboard shortcut for adding reactions to last message in channel or thread (#4478) * added shortcut in modal * lk file * ran i18n extract * i18n * added the constants * added en translations * added the ability to add last posts reaction to center text input * added e2e tests for center area shortcut * lint * new tes * short comment added * added RHS shortcut for last message reaction * fixed rhs and center shortcut * added comments to def posts * clear timeouts in comp unmount * update snapshot * fixed tests * added tests * new lines at the end of the file * Spell check in comments * reverted underscores * spel in test files * more spell check in test files * added mac and win keyboard control button to shortcut * trailing spc removed * func name change * new proposal change added as per comment #4478 * complicated issue regarding closing of shortcut emetter removed and simplified * simplified action for emoji for last post * proptypes changes after recent modifications * addressed issue of improper proptypes, fixed point 2 of mattermost-webapp/pull/4478#pullrequestreview-339313236 for rhs comment * fixed race condition to dissallow shortcut for mobile, system and other similar post types * for cleaner git diff * for cleaner git diff * var name change * more var name changes * more more var changes * all var names updated as per @deanwhillier comments * updated tests * rever package-lock * blocked shortcut when post not in view * hide shortcut when executed on post not visible to user * - Reverted to old spec where in the shortcut should be executed when focus is not on center - Blocked the shortcut when any modals or popups are open * blocked the shortcut for dates, welcome messages and other non message posts * fixed tests * some minor code beautifications * moved e2e test under emoji folder * improved existing tests * added more tests * minor typo * removed commented tests * fix failing type tests * changin few tests * modified tests after test review * lint check * removed the one test * Update components/rhs_comment/rhs_comment.jsx comment Co-Authored-By: Sudheer <[email protected]> * Update comment in components/post_view/post_list_row/post_list_row.jsx Co-Authored-By: Sudheer <[email protected]> * common function used Co-authored-by: Sudheer <[email protected]> * MM-22309 - Updating marketplace modal css (#4908) * MM-22309 - Updating marketplace modal css * Updating test * Updating css * Promote Dutch and Russian to Beta, v5.22 (#4957) * Promote Dutch and Russian to Beta, v5.22 Both have exceeded 90% in translation server https://translate.mattermost.com/projects/mattermost/ Big shoutouts to Tom De Moor and flynbit! * Update schema_admin_settings.test.jsx.snap * [MM-12330] Add "Unarchive Channel" option to archived channels menu (#4458) Automatic Merge * [MM-22294] Fixed the use of handleUploadError on create_comment (#4875) Co-authored-by: mattermod <[email protected]> * MM-19755 Add landmark roles (#4522) * MM-19755 Add landmark roles * fix eslint * restore snapshot * fix current issues * fix axe's "Ensures landmarks are unique" eror * fix linter * update snapshot Co-authored-by: mattermod <[email protected]> * MM-22634: Add Team Name back to team list (#4962) * Add Team Name back to team list * add snapshot test * Change message for invite code in team settings (#4940) * [MM-20594] Migrate components/more_direct_channels to TypeScript (#4846) * [MM-20594] Migrate components/more_direct_channels to TypeScript * onHide is optional * Lank bline * Merge'd Co-authored-by: mattermod <[email protected]> * [MM-22310] Remove back button when user has no accounts (#4946) * [MM-22310] Remove back button when user has no accounts * Change condition logic * Add types for webapp-specific state (#4910) * Add types for webapp-specific state * Update mattermost-redux * Update Redux commit hash (#4928) * Fix mention autocomplete overlap spec (#4956) * Fix mention autocomplete overlap spec * Fix lint * fixed typo defauleMessage -> defaultMessage (#4970) * Added test to verify Accessibility Support in Account Settings (#4944) * Added Cypress test to verify Accessibility Support in Account Settings * Updated fixture * Dummy push to trigger the build process Co-authored-by: mattermod <[email protected]> * Added Cypress test to verify Accessibility Support in Modals & Dialogs (#4937) * Added test to verify Accessibility Support in Modals & Dialogs * Corrected comments Co-authored-by: mattermod <[email protected]> * Added Cypress test to verify Accessibility Support in Dropdown Menus (#4951) * Minor fixes based on run on localhost * Added Cypress test to verify accessibility support in Dropdown Menus * Added step to verify if menu is closed when we press Escape * Handled a case where an Open Team was not available to join and test was failing * remove invalid/outdated instruction and link (#4966) * [MM-20546]Migrate 'components/user_settings/security/mfa_section' module and associated tests to TypeScript #13695 * Fix lint check * Fix lint check fail * [MM-22622] Add mentionHighlightDisabled props on create post and create comment (#4916) * MM-22622 Send mentionHighlighDisabled props when creating post or comment without highlights * MM-22622 Disable mention highlight on system header change message * MM-22622 Fix snapshot tests * fix channel modal not showing updated roles (#4938) * [MM-20518] - Convert channel_groups_manage_modal to TypeScript (#4900) * covnert channel_groups_manage_modal to TypeScript * convert channel_groups_manage_modal to TypeScript * Fix eslint errors * Make requested changes * Fix eslint errors * Change method param to use lowerCamelCase Co-authored-by: mattermod <[email protected]> * Fix lint failed check * [GH-8665/MM-9832]: Refactor utility functions that use getState and upd… (#4402) * [GH-8665] [WIP]: Refactor utility functions that use getState and update components * Resolve PR comments and linter * Rebase with master and fix component failing tests * Fix util failing tests and adapt components * Fix failing tests and clean up * Resolve second batch of PR comments * Resolve MR comments and fix lint and tests * Fix remaining linter issues * Fix create_comment.test.jsx * Add missing reference to currentUserId in focusPost. * Please linter * Please linter * Please linter * Make channel test pass * Fix QA errors * Fix type error * Add removed change after merge * Fix test for handleUnicodeEmoji * Fix JS errors after integration tests * Fix image preview * Resolve PR comments * Remove yarn.lock * remove redundant parameter * Fix formatting in renderer Co-authored-by: Harrison Healey <[email protected]> * MM-22969 - Updating paragraph styling in list item (#4981) * Use webapp-specific GlobalState everywhere (#4965) * MM-23012 - Hiding scrollbars in IE from LHS/RHS (#4982) * Upgrade to cypress 4.0 (#4954) * Upgrade to cypress 4.0 * Added browser to test report * Added electron to scripts * Fix lint * Made chrome default * Provided default chrome headless * Fix lint Co-authored-by: mattermod <[email protected]> * Added step to ensure that new users are added to default team ad-1 (#4987) * MM-22754 Fix for missing closing bracket (#4988) * MM-22760 - Updating position logic for post menu (#4979) * MM-22760 - Updating position logic for post menu * Removing console * Updating test * Updating test * MM-11505 Update permalink view (#4825) * MM-11505 Update permalink view * Use existing channel view for permalinks * Add a nre URL route so channelView can be used for permalinks * Add new history toast * Remove unused strings * Update tests * * Add a redirect to channel view from permalink * Remove the dependency of focussed view state from redux * Add a state in react to keep track of permalink view * Fix i18 * * Add unit tests * Change timeout for highlight * Update cypress tests * Add couple of constants Co-authored-by: mattermod <[email protected]> * GH-13807 Enable notification sounds in FF, update i18n strings with same (#4959) * enable notification sounds in FF, update i18n strings with same * #13807 update .jsx.snap to pass tests * gh-13807 revert i18n json files and package-lock.json per feedback * gh-13807 revert package-lock.json again * gh-13807 revert package-lock.json again again * gh-13807 revert i18n json files and package-lock.json per feedback, 4th time is the charm * gh-13807 revert package-lock.json per feedback, 5th time is the charm * gh-13807 revert package-lock.json per feedback, 7th time is the charm, no really * gh-13807 revert i18n json files per feedback, 7th time is the charm, like totally for sure * gh-13807 remove excessive parens per feedback for legibility * fix highlighting of at mentions of self (#4994) (#4997) (cherry picked from commit 2a78ea9) * MM-22929 - Updating back button in Account modal (#4980) * MM-22929 - Updating back button in Account modal * Updating test * Fix mochawesome merge (#5001) * MM-22698 Fix for unread channel showing Loading indicator (#5002) * Update react window hash * Update package lock (#5004) * Fix type error for user.mfa_active * Update action type and test cases for mfa_section * Fix lint check failure * Fix CI test failure * fix identation lint check * Reverst package-lock.json file change * Revert package log change Co-authored-by: mattermod <[email protected]> Co-authored-by: Nev Angelova <[email protected]> Co-authored-by: Rohitesh Gupta <[email protected]> Co-authored-by: Harrison Healey <[email protected]> Co-authored-by: Yusuke Nemoto <[email protected]> Co-authored-by: Amy Blais <[email protected]> Co-authored-by: Md Zubair Ahmed <[email protected]> Co-authored-by: Sudheer <[email protected]> Co-authored-by: Asaad Mahmood <[email protected]> Co-authored-by: Jason Blais <[email protected]> Co-authored-by: Allen Lai <[email protected]> Co-authored-by: Devin Binnie <[email protected]> Co-authored-by: Vladimir Lebedev <[email protected]> Co-authored-by: Scott Bishel <[email protected]> Co-authored-by: Mario de Frutos Dieguez <[email protected]> Co-authored-by: Joseph Baylon <[email protected]> Co-authored-by: Doug Lauder <[email protected]> Co-authored-by: Farhan Munshi <[email protected]> Co-authored-by: Jason Frerich <[email protected]> Co-authored-by: Saturnino Abril <[email protected]> Co-authored-by: Ths2-9Y-LqJt6 <[email protected]>

…te comment (mattermost#4916) * MM-22622 Send mentionHighlighDisabled props when creating post or comment without highlights * MM-22622 Disable mention highlight on system header change message * MM-22622 Fix snapshot tests

MM-22622 Send mentionHighlighDisabled props when creating post or com…

e1b9e48

…ment without highlights

fmunshi added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester 1: UX Review Requires review by a UX Designer labels Feb 16, 2020

fmunshi requested review from srkgupta, hmhealey, sudheerDev and michaelgamble February 16, 2020 18:09

hmhealey reviewed Feb 23, 2020

View reviewed changes

srkgupta mentioned this pull request Feb 24, 2020

[MM-22622] Add Ephemeral response when using mentions without permissions mattermost/mattermost#13902

Merged

Merge branch 'master' into MM-22622-Check-mentionHighlightDisabled-in…

bcf8a0f

…side-markdown

hmhealey approved these changes Feb 28, 2020

View reviewed changes

srkgupta suggested changes Mar 2, 2020

View reviewed changes

fm2munsh added 2 commits March 2, 2020 20:12

MM-22622 Disable mention highlight on system header change message

270a9d4

MM-22622 Fix snapshot tests

3fca2db

srkgupta approved these changes Mar 3, 2020

View reviewed changes

srkgupta added QA Review Done and removed 3: QA Review Requires review by a QA tester labels Mar 3, 2020

sudheerDev approved these changes Mar 4, 2020

View reviewed changes

sudheerDev removed the 2: Dev Review Requires review by a core commiter label Mar 4, 2020

hanzei added this to the v5.22.0 milestone Mar 4, 2020

michaelgamble approved these changes Mar 4, 2020

View reviewed changes

fmunshi removed the 1: UX Review Requires review by a UX Designer label Mar 4, 2020

fmunshi merged commit 0374bcf into mattermost:master Mar 4, 2020

fmunshi added the 4: Reviews Complete All reviewers have approved the pull request label Mar 4, 2020

sbishel mentioned this pull request Mar 6, 2020

MM-22970: Fix highlighting of at mentions of self #4994

Merged

amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Mar 13, 2020

fmunshi mentioned this pull request Apr 21, 2020

[MM-24177] Don't highlight group mention if doesn't have required permission #5372

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MM-22622] Add mentionHighlightDisabled props on create post and create comment #4916

[MM-22622] Add mentionHighlightDisabled props on create post and create comment #4916

fmunshi commented Feb 16, 2020

hmhealey Feb 23, 2020

fmunshi Feb 24, 2020 •

edited

Loading

srkgupta left a comment

fmunshi commented Mar 2, 2020

fmunshi commented Mar 2, 2020

srkgupta commented Mar 2, 2020

fmunshi commented Mar 3, 2020

srkgupta commented Mar 3, 2020

srkgupta left a comment

sudheerDev left a comment

michaelgamble left a comment

[MM-22622] Add mentionHighlightDisabled props on create post and create comment #4916

[MM-22622] Add mentionHighlightDisabled props on create post and create comment #4916

Conversation

fmunshi commented Feb 16, 2020

Summary

Ticket Link

Related Pull Requests

Screenshots

hmhealey Feb 23, 2020

Choose a reason for hiding this comment

fmunshi Feb 24, 2020 • edited Loading

Choose a reason for hiding this comment

srkgupta left a comment

Choose a reason for hiding this comment

fmunshi commented Mar 2, 2020

fmunshi commented Mar 2, 2020

srkgupta commented Mar 2, 2020

fmunshi commented Mar 3, 2020

srkgupta commented Mar 3, 2020

srkgupta left a comment

Choose a reason for hiding this comment

sudheerDev left a comment

Choose a reason for hiding this comment

michaelgamble left a comment

Choose a reason for hiding this comment

fmunshi Feb 24, 2020 •

edited

Loading