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

MM-27165 - Updating post menu css #6080

Merged
merged 6 commits into from
Sep 10, 2020
Merged

MM-27165 - Updating post menu css #6080

merged 6 commits into from
Sep 10, 2020

Conversation

asaadmahmood
Copy link
Contributor

@asaadmahmood asaadmahmood commented Aug 6, 2020 •
edited
Loading

Summary

MM-27165 - Updating post menu css
MM-26860 - Bot username spacing issue

Ticket Link

https://mattermost.atlassian.net/browse/MM-27165
https://mattermost.atlassian.net/browse/MM-26860

Screenshots

Post menu css
Screenshot 2020-08-06 at 8 16 45 PM

Bots in thread
Screenshot 2020-08-20 at 3 46 10 PM

@asaadmahmood asaadmahmood added the 1: UX Review Requires review by a UX Designer label Aug 6, 2020
@asaadmahmood asaadmahmood reopened this Aug 20, 2020
@asaadmahmood asaadmahmood added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Aug 21, 2020
andrewbrown00
andrewbrown00 approved these changes Aug 21, 2020
View reviewed changes
Copy link

@andrewbrown00 andrewbrown00 left a comment

Choose a reason for hiding this comment

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

Placement feels good, thanks @asaadmahmood 🎉

@andrewbrown00 andrewbrown00 removed the 1: UX Review Requires review by a UX Designer label Aug 21, 2020
@asaadmahmood asaadmahmood added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Aug 21, 2020
hmhealey
hmhealey reviewed Aug 25, 2020
View reviewed changes
components/post_view/post/post.jsx
@@ -229,7 +229,7 @@ class Post extends React.PureComponent {
}

let rootUser = '';
if (this.hasSameRoot(this.props) && !fromBot) {
Copy link
Member

@hmhealey hmhealey Aug 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This was added to fix https://mattermost.atlassian.net/browse/MM-15228 from what I can tell. Can you confirm that this isn't going to cause a regression in that ticket?

Also, this might be a good place to add a test, if possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, it definitely causes an issue there. I'll fix that and this ticket together.

@asaadmahmood
Copy link
Contributor Author

@hmhealey Should be fixed.
Screenshot 2020-08-26 at 3 26 43 PM

hmhealey
hmhealey approved these changes Aug 27, 2020
View reviewed changes
components/rhs_comment/rhs_comment.jsx Outdated Show resolved Hide resolved
@hmhealey hmhealey removed the 2: Dev Review Requires review by a core commiter label Aug 27, 2020
@asaadmahmood asaadmahmood assigned ogi-m and unassigned ogi-m Sep 1, 2020
@asaadmahmood asaadmahmood removed the request for review from ogi-m September 9, 2020 08:18
@srkgupta srkgupta self-requested a review September 9, 2020 15:40
srkgupta
srkgupta suggested changes Sep 9, 2020
View reviewed changes
Copy link
Contributor

@srkgupta srkgupta left a comment

Choose a reason for hiding this comment

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

Hi @asaadmahmood

The first part of the issue, i.e. Post hover menu partially hides post text still needs to be fixed when the Display is set as Compact View. Please check out the screenshot of the issue seen:

Screenshot 2020-09-09 at 9 12 42 PM

The second issue is fixed and is working fine now.

@asaadmahmood
Copy link
Contributor Author

@srkgupta That case is intentionally left out, because on a normal post, the reply icon sits on the right, I cannot place the post menu on top, because then there will be a shift in where the reply icon appears.

Screenshot 2020-09-10 at 8 08 22 PM

So we can accept it. Once we have a different solution for the reply icon and the number, we can move the post menu up all the time.

srkgupta
srkgupta approved these changes Sep 10, 2020
View reviewed changes
Copy link
Contributor

@srkgupta srkgupta left a comment

Choose a reason for hiding this comment

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

Approving the PR based on the above comment. Thanks @asaadmahmood

@srkgupta srkgupta added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester Setup Cloud Test Server Setup a test server using Mattermost Cloud labels Sep 10, 2020
@mm-cloud-bot
Copy link

Test server destroyed

@asaadmahmood asaadmahmood merged commit 82ade4e into mattermost:master Sep 10, 2020
@asaadmahmood asaadmahmood deleted the MM-27165 branch September 10, 2020 15:13
@amyblais amyblais added this to the v5.28 milestone Sep 10, 2020
@amyblais amyblais added Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation labels Sep 15, 2020
jfrerich pushed a commit that referenced this pull request Oct 23, 2020
@asaadmahmood @jfrerich
MM-27165 - Updating post menu css (#6080)
d6d8d2f 
* MM-27165 - Updating post menu css

* Updating bot post thread view

* Removing fromBot parameter

* Updating bot post profile pic and header

* Removing debugging code

* Removing console
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@srkgupta srkgupta srkgupta approved these changes

@hmhealey hmhealey hmhealey approved these changes

@andrewbrown00 andrewbrown00 andrewbrown00 approved these changes

@devinbinnie devinbinnie devinbinnie approved these changes

Assignees
No one assigned
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation
Projects
None yet
Milestone
v5.28.0
Development

Successfully merging this pull request may close these issues.
8 participants
@asaadmahmood @mm-cloud-bot @srkgupta @hmhealey @andrewbrown00 @devinbinnie @ogi-m @amyblais