-
Notifications
You must be signed in to change notification settings - Fork 2.7k
[MM 23264] Show Confirmation Modal for group mentions #5065
[MM 23264] Show Confirmation Modal for group mentions #5065
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looked over the screens with Hossein, everything looks good to go on this one from my perspective.
c9a4dfd
to
37deb4c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌
values={Object {}} | ||
/> | ||
} | ||
title="" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are these emptied (throughout the file)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because prior to my change, the title were always getting set regardless if the confirmation modal was being shown or not. In my change, I start them off as blank and only set them if needed. So they don't won't always have values associated to them unless we will display the confirmation modal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some suggestions/questions but looks great otherwise.
5099a43
to
3a5a6dd
Compare
make warning dialogue appear for group mentions and all channel mentions fix linting and i18n extract address UX concerns regarding wording for group dialogue fix lint and add tests address PR comments update text in UI to include group mentions
63fafb5
to
d4f69ab
Compare
Summary
Show the confirmation modal for group mentions
Ticket Link
https://mattermost.atlassian.net/browse/MM-23264
Related Pull Requests
mattermost/mattermost#14068
mattermost/mattermost-redux#1086