Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

[MM-18074] Format of Position in User Profile is incorrect #3785

Merged
merged 5 commits into from
Nov 1, 2019
Merged

[MM-18074] Format of Position in User Profile is incorrect #3785

merged 5 commits into from
Nov 1, 2019

Conversation

bradjcoughlin
Copy link
Contributor

@bradjcoughlin bradjcoughlin commented Sep 27, 2019 •
edited
Loading

Summary

The format of the position string of text looks slightly off from the design. I'm not sure if this is the best fix, but to me, but increasing the size by a pixel (15px) looks a bit better:
Screen Shot 2019-09-30 at 11 03 45 AM

Ticket Link

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

@bradjcoughlin bradjcoughlin added the 1: PM Review Requires review by a product manager label Sep 27, 2019
@bradjcoughlin bradjcoughlin added this to the v5.16.0 milestone Sep 27, 2019
@amyblais amyblais added the CherryPick/Approved Meant for the quality or patch release tracked in the milestone label Sep 27, 2019
@bradjcoughlin bradjcoughlin added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Sep 30, 2019
@wiersgallak
Copy link
Contributor

@bradjcoughlin I'll deter to @michaelgamble on the exact pixels, but it looks to have more vertical white space above the position string then below - I'd expect it to be the same or a bit closer to the top than bottom

Screen Shot 2019-09-30 at 1 17 02 PM

michaelgamble
michaelgamble suggested changes Oct 6, 2019
View reviewed changes
Copy link
Contributor

@michaelgamble michaelgamble left a comment

Choose a reason for hiding this comment

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

@wiersgallak observation is correct. The spacing between the users fullname and role should be equal (both should be 12px, it seems the block below the blue is sitting with a top margin of 20px, that should get adjusted to 12px.

@amyblais amyblais modified the milestones: v5.16.0 , v5.17.0 Oct 15, 2019
@bradjcoughlin
Copy link
Contributor Author

@michaelgamble I've reduced the font size back down to 14px, and changed the padding/margin so that the white space appears equidistant.

wiersgallak
wiersgallak approved these changes Oct 23, 2019
View reviewed changes
Copy link
Contributor

@wiersgallak wiersgallak left a comment

Choose a reason for hiding this comment

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

Approve for the position of the "Position" field. There is a cut-ff of the profile picture, but this bug can be filed separately.

@wiersgallak wiersgallak removed the 1: PM Review Requires review by a product manager label Oct 28, 2019
@bradjcoughlin
CSS adjustments
7cffc8d 
- Reduce padding on position to 12px
- Icon margin right 8px
- Icon opacity to .6
- User ID 12px
- Add 2px stroke to profile image
@bradjcoughlin
Merge branch 'master' into MM-18074
ca86beb
@mattermod
Copy link
Contributor

Mattermost test server updated with git commit ca86beb98730147052784681c013fa76489a8102.

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

@bradjcoughlin
Copy link
Contributor Author

@michaelgamble Implemented changes discussed offline. I'll file a new ticket to add the "role" icon.

@bradjcoughlin bradjcoughlin requested a review from a team October 30, 2019 17:49
@ghost ghost requested review from gabrieljackson and sbishel and removed request for a team October 30, 2019 17:49
@bradjcoughlin bradjcoughlin added the 2: Dev Review Requires review by a core commiter label Oct 30, 2019
michaelgamble
michaelgamble approved these changes Oct 30, 2019
View reviewed changes
Copy link
Contributor

@michaelgamble michaelgamble left a comment

Choose a reason for hiding this comment

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

Took a look over the changes based on our last discussion, looks like everything is how it should be :)

@lindalumitchell
Copy link
Contributor

Not sure if this had official QA review, but I took a look and this LGTM.

@lindalumitchell lindalumitchell added QA Review Done Tests/Not Needed Does not require new release tests labels Oct 31, 2019
@amyblais amyblais removed the CherryPick/Approved Meant for the quality or patch release tracked in the milestone label Nov 1, 2019
@amyblais amyblais modified the milestones: v5.17.0, v5.18.0 Nov 1, 2019
gabrieljackson
gabrieljackson approved these changes Nov 1, 2019
View reviewed changes
Copy link
Contributor

@gabrieljackson gabrieljackson left a comment

Choose a reason for hiding this comment

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

LGTM!

@gabrieljackson gabrieljackson added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Nov 1, 2019
@bradjcoughlin bradjcoughlin merged commit be7cc66 into mattermost:master Nov 1, 2019
@bradjcoughlin bradjcoughlin deleted the MM-18074 branch November 1, 2019 19:13
@mattermod
Copy link
Contributor

Test server destroyed

@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Nov 4, 2019
brewsterbhg pushed a commit to brewsterbhg/mattermost-webapp that referenced this pull request Nov 11, 2019
@bradjcoughlin @brewsterbhg
[MM-18074] Format of Position in User Profile is incorrect (mattermos…
f3a32e3 
…t#3785)

- Reduce padding on position to 12px
- Icon margin right 8px
- Icon opacity to .6
- User ID 12px
- Add 2px stroke to profile image
@hanzei hanzei removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Nov 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gabrieljackson gabrieljackson gabrieljackson approved these changes

@sbishel sbishel sbishel approved these changes

@michaelgamble michaelgamble michaelgamble approved these changes

@wiersgallak wiersgallak wiersgallak approved these changes

Assignees

@wiersgallak wiersgallak

Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation QA Review Done Tests/Not Needed Does not require new release tests
Projects
None yet
Milestone
v5.18.0
9 participants
@bradjcoughlin @wiersgallak @mattermod @lindalumitchell @gabrieljackson @sbishel @michaelgamble @hanzei @amyblais