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

[Gekidou] Groups mentions / highlights in messages (posts) #6338

Merged
merged 14 commits into from
Jun 28, 2022
Merged

[Gekidou] Groups mentions / highlights in messages (posts) #6338

merged 14 commits into from
Jun 28, 2022

Conversation

shaz-r
Copy link
Contributor

@shaz-r shaz-r commented Jun 3, 2022 •
edited
Loading

Summary

This displays the correct styling for a group mention.

Note: This does NOT check if the current user is part of the group, and highlight accordingly. That will come in the next PR (with saving the group memberships). Edit: PR Here

Screen Shot 2022-06-09 at 2 09 51 pm

Screen Shot 2022-06-09 at 2 12 00 pm

Ticket Link

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

Checklist

  • Added or updated unit tests (required for all new features)
  • Has UI changes
  • Includes text changes and localization file updates
  • Have tested against the 5 core themes to ensure consistency between them.

Device Information

This PR was tested on:

Screenshots

Release Note

@shaz-r shaz-r self-assigned this Jun 3, 2022
@shaz-r shaz-r added Work In Progress Not yet ready for review Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) labels Jun 3, 2022
Base automatically changed from MM-43983-Autocomplete-local-fetches to gekidou June 6, 2022 21:55
enahum
enahum reviewed Jun 9, 2022
View reviewed changes
app/actions/remote/groups.ts Outdated Show resolved Hide resolved
@shaz-r shaz-r requested review from larkox and avinashlng1080 June 9, 2022 04:07
@shaz-r shaz-r removed Work In Progress Not yet ready for review Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) labels Jun 9, 2022
@shaz-r shaz-r changed the title [Gekidou] [WIP] Groups Mention in Post [Gekidou] Groups Mention in Post Jun 9, 2022
@shaz-r shaz-r added the Work In Progress Not yet ready for review label Jun 9, 2022
@shaz-r shaz-r added 2: Dev Review Requires review by a core commiter and removed Work In Progress Not yet ready for review labels Jun 10, 2022
@shaz-r shaz-r marked this pull request as ready for review June 10, 2022 03:04
@shaz-r shaz-r requested a review from enahum June 10, 2022 03:04
@enahum enahum mentioned this pull request Jun 11, 2022
Merged
enahum
enahum requested changes Jun 11, 2022
View reviewed changes
app/helpers/database/index.ts Outdated Show resolved Hide resolved
app/actions/remote/groups.ts Outdated Show resolved Hide resolved
app/actions/remote/groups.ts Outdated Show resolved Hide resolved
app/actions/remote/groups.ts Outdated Show resolved Hide resolved
app/actions/remote/groups.ts Outdated Show resolved Hide resolved
app/actions/remote/user.ts Outdated Show resolved Hide resolved
launch.json Outdated Show resolved Hide resolved
@avinashlng1080
Copy link
Contributor

I did a first run at the review and apart from Elias' comments, I did not find anything. I will do another run after the corrections.

larkox
larkox approved these changes Jun 13, 2022
View reviewed changes
Copy link
Contributor

@larkox larkox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking, just a few minor suggestions.

app/actions/remote/user.ts Outdated Show resolved Hide resolved
app/actions/remote/user.ts Outdated Show resolved Hide resolved
app/components/markdown/at_mention/at_mention.tsx Show resolved Hide resolved
app/components/markdown/at_mention/at_mention.tsx Show resolved Hide resolved
app/components/markdown/at_mention/at_mention.tsx Outdated Show resolved Hide resolved
@shaz-r shaz-r requested review from enahum and larkox June 20, 2022 10:20
larkox
larkox approved these changes Jun 20, 2022
View reviewed changes
Copy link
Contributor

@larkox larkox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking, but a few things that probably could use some extra thought.

app/actions/local/group.ts Outdated Show resolved Hide resolved
app/actions/local/group.ts Show resolved Hide resolved
app/actions/remote/groups.ts Show resolved Hide resolved
avinashlng1080
avinashlng1080 suggested changes Jun 21, 2022
View reviewed changes
Copy link
Contributor

@avinashlng1080 avinashlng1080 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - just correct the try/catch blocks as Daniel suggested.

.gitignore Show resolved Hide resolved
@shaz-r shaz-r requested a review from larkox June 21, 2022 22:59
@shaz-r
Copy link
Contributor Author

shaz-r commented Jun 21, 2022 •
edited
Loading

@larkox / @enahum - moved the databaseManager.database callers into their own try/catch blocks.

@shaz-r shaz-r requested a review from avinashlng1080 June 23, 2022 22:02
enahum
enahum requested changes Jun 23, 2022
View reviewed changes
app/actions/local/group.ts Outdated Show resolved Hide resolved
app/actions/local/group.ts Outdated Show resolved Hide resolved
app/actions/local/group.ts Outdated Show resolved Hide resolved
app/actions/remote/groups.ts Show resolved Hide resolved
app/actions/remote/user.ts Outdated Show resolved Hide resolved
avinashlng1080
avinashlng1080 suggested changes Jun 27, 2022
View reviewed changes
app/actions/local/group.ts Outdated Show resolved Hide resolved
@shaz-r shaz-r requested review from avinashlng1080 and enahum June 28, 2022 09:57
@shaz-r shaz-r changed the title [Gekidou] Groups Mention in Post [Gekidou] Groups mentions / highlights in messages (posts) Jun 28, 2022
@shaz-r shaz-r merged commit f8140f2 into gekidou Jun 28, 2022
@shaz-r shaz-r deleted the MM-44672-Gekidou-Post-Mention-Groups-Fetch branch June 28, 2022 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@enahum enahum enahum approved these changes

@larkox larkox larkox approved these changes

@avinashlng1080 avinashlng1080 Awaiting requested review from avinashlng1080

Assignees

@shaz-r shaz-r

Labels
2: Dev Review Requires review by a core commiter release-note
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@shaz-r @avinashlng1080 @larkox @enahum @mm-cloud-bot