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.

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

[MM-18146] Order tooltip for reactions on web by who recently reacted with those emoji #3671

Merged
merged 5 commits into from Sep 24, 2019

Conversation

@abdusabri
Copy link
Contributor

abdusabri commented Sep 15, 2019

Summary

In the tooltip of an emoji reaction to a post, the users are ordered by recency (who reacted most recently) who reacted first, not alphabetically. And if the logged-in user has reacted, "You" - or its localized equivalent - will precede the other users in the tooltip (even if they reacted after before the logged-in user).

Ticket Link

Fixes mattermost/mattermost-server#12186

Copy link
Member

esethna left a comment

Nice work @abdusabri!

@esethna esethna removed the 1: PM Review label Sep 16, 2019
@esethna esethna requested a review from hmhealey Sep 16, 2019
@esethna esethna added this to the v5.18.0 milestone Sep 16, 2019
@esethna esethna changed the title [GH-12186] Order tooltip for reactions on web by who recently reacted with those emoji [MM-18146] Order tooltip for reactions on web by who recently reacted with those emoji Sep 16, 2019
@hmhealey hmhealey requested a review from bradjcoughlin Sep 20, 2019
@hmhealey

This comment has been minimized.

Copy link
Member

hmhealey commented Sep 20, 2019

The code looks great, but I want to double check the ordering with @esethna. Based on the ticket, I would've expected that the list starts with the user who reacted first, but this has them appearing last.

Copy link
Member

esethna left a comment

@hmhealey you are correct, good catch. @abdusabri can we please reverse the order so the first person who reacted with the emoji appears first in the tooltip, and the most recent person appears last. Apologies the ticket should have been more clear.

@abdusabri

This comment has been minimized.

Copy link
Contributor Author

abdusabri commented Sep 21, 2019

Thanks @hmhealey for the feedback!

@esethna , no problem 🙂, updated.

@mattermod

This comment has been minimized.

Copy link
Contributor

mattermod commented Sep 23, 2019

Mattermost test server updated with git commit 48cde007f1b0192f49a86d337efc59d22ecf9288.

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

@hmhealey hmhealey requested a review from esethna Sep 23, 2019
Copy link
Member

hmhealey left a comment

Awesome, thanks for changing that. I'll bump Eric for a re-review so we can get this in

@hmhealey hmhealey removed the 2: Dev Review label Sep 23, 2019
@hmhealey hmhealey requested a review from saturninoabril Sep 23, 2019
@esethna

This comment has been minimized.

Copy link
Member

esethna commented Sep 23, 2019

Thanks @abdusabri, nice work!

@bradjcoughlin bradjcoughlin requested a review from ogi-m Sep 24, 2019
users,
['You', 'username_2', 'username_3']
);
});

This comment has been minimized.

Copy link
@saturninoabril
Copy link
Member

saturninoabril left a comment

Tested and passed.

Thanks @abdusabri!

@mattermod

This comment has been minimized.

Copy link
Contributor

mattermod commented Sep 24, 2019

Test server destroyed

@saturninoabril saturninoabril merged commit 307ccc6 into mattermost:master Sep 24, 2019
11 checks passed
11 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: build-docker Your tests passed on CircleCI!
Details
ci/circleci: i18n-check Your tests passed on CircleCI!
Details
ci/circleci: install Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: prepare-docker-build Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
ci/circleci: upload-s3 Your tests passed on CircleCI!
Details
cla/mattermost abdusabri authorized
Details
mattermost/serverless-ops/github PR is up-to-date.
untagged-build Workflow: untagged-build
Details
jwilander added a commit that referenced this pull request Sep 24, 2019
… with those emoji (#3671)

* Order tooltip for reactions by who recently reacted

* Improve sorting by recency test for reactions

* Order reactions by who reacted first (instead of who reacted last)
saturninoabril added a commit to saturninoabril/mattermost-webapp that referenced this pull request Sep 27, 2019
… with those emoji (mattermost#3671)

* Order tooltip for reactions by who recently reacted

* Improve sorting by recency test for reactions

* Order reactions by who reacted first (instead of who reacted last)
skheria added a commit to uber-uchat/mattermost-webapp that referenced this pull request Oct 3, 2019
… with those emoji (mattermost#3671)

* Order tooltip for reactions by who recently reacted

* Improve sorting by recency test for reactions

* Order reactions by who reacted first (instead of who reacted last)
skheria added a commit to uber-uchat/mattermost-webapp that referenced this pull request Oct 3, 2019
… with those emoji (mattermost#3671)

* Order tooltip for reactions by who recently reacted

* Improve sorting by recency test for reactions

* Order reactions by who reacted first (instead of who reacted last)
skheria added a commit to uber-uchat/mattermost-webapp that referenced this pull request Oct 3, 2019
… with those emoji (mattermost#3671)

* Order tooltip for reactions by who recently reacted

* Improve sorting by recency test for reactions

* Order reactions by who reacted first (instead of who reacted last)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.