MM-22894/MM-23046/MM-24274/MM-24375 Quick wins bucket list 5 #5353

asaadmahmood · 2020-04-19T09:58:49Z

Summary

Quick wins bucket list 5
Escape to close the expanded RHS is not in, as there are a lot of corner cases around that.

Ticket Link

https://mattermost.atlassian.net/browse/MM-22894
https://mattermost.atlassian.net/browse/MM-23046
https://mattermost.atlassian.net/browse/MM-24375

Screenshots

mattermod · 2020-04-19T16:30:51Z

Creating a new SpinWick test server using Mattermost Cloud.

mattermod · 2020-04-19T16:36:04Z

Mattermost test server created! 🎉

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

Account Type	Username	Password
Admin	sysadmin	Sys@dmin123
User	user-1	User-1@123

esethna · 2020-04-19T16:46:55Z

Nice!

We can probably remove this text now:
Channel name in the RHS header is missing a click effect
Do we want to include the changes to the "Add comment" button in this PR?
0/5 on the changes to the RHS expanded state in terms of the UI? If you think we can do better, feel free to change what was written in the ticket
Can we allow "ESC" to collapse the expanded RHS?
Remove the expand button in mobile web view
The channel name button doesn't work in mobile web view

asaadmahmood · 2020-04-19T17:29:51Z

Done
Done
We should do that globally in a separate PR.
I am concerned about the full opacity channel header at the top not vertically aligning with the full opacity RHS below it. I'll see what others think before making any changes.
That may have a bunch of corner cases, so I would avoid doing that in this PR.
Done.
Do we want it to close the sidebar? We already have another way to close the sidebar on mobile. For most cases, a user will already be in the current channel, unless he is jumping from the search bar, in which case, he can jump to the channel from the search results, or we can also add Jump to be very explicit. I don't want it to function in different ways on mobile and desktop, with it closing it on mobile, but not on the desktop app. Especially since its not clear that clicking it will close the sidebar and you will lose the thread you were viewing. You would have to either search for it again from search, or manually search for it in the channel.

mattermod · 2020-04-19T17:43:02Z

Test server creation failed. See the logs for more information.

matthewbirtch

Looks great @asaadmahmood. Thank you!

Love that we've animated the transition from compact to expanded state on the RHS. This really helps a lot. Nice call on that.
For the hover state of the channel name in the Thread RHS header, can we darken the actual text on hover as well? Maybe we follow the same convention for the icons, so go with 56% center channel color as a default state, 72% center channel color as a hover state? I'll update the design files if we're aligned to that.
The expanded state of the RHS header has a rounded top-left corner. Is that intentional? I see that was requested in the ticket, but it feels a little odd to me since no other corners for the panel are rounded
For the expanded state of the RHS, I wonder if we should have a max-width set on it? On a large screen it's so wide. I'm thinking something like 960px wide as a max.
As per the note from @esethna, it looks like the expand button is still there in mobile webview, but the close button is now gone. Can we remove the expand button and bring back the close?

asaadmahmood · 2020-04-20T07:06:33Z

@matthewbirtch I did update that, both the close button and the expand button aren't there, and we don't have a close button because the RHS is closed in the mobile web view with the back button at the top beside the search bar.

The PR just did not update automatically, which is why you can't see it right now.

asaadmahmood · 2020-04-20T07:36:30Z

@matthewbirtch

Cool.
Done.
Wasn't a fan of that, so removed it.
Done. Good idea.
As mentioned above.

matthewbirtch · 2020-04-20T15:27:51Z

Thanks @asaadmahmood. I think you may have tagged the wrong github user in your comment above :)

Things are looking good. However, I noticed now that we've added a max-width on the expanded state of the RHS, I think it messed up the animation :(

asaadmahmood · 2020-04-20T15:52:24Z

@matthewbirtch Will fix that.

esethna · 2020-04-20T16:57:07Z

Do we want it to close the sidebar? We already have another way to close the sidebar on mobile. For most cases, a user will already be in the current channel, unless he is jumping from the search bar, in which case, he can jump to the channel from the search results, or we can also add Jump to be very explicit. I don't want it to function in different ways on mobile and desktop, with it closing it on mobile, but not on the desktop app. Especially since its not clear that clicking it will close the sidebar and you will lose the thread you were viewing. You would have to either search for it again from search, or manually search for it in the channel.

@asaadmahmood I think either we keep the button and it should close the sidebar and take the user to that channel or we just make it not clickable in the mobile view. 0/5

Also I think there's a bug with expanding: https://i.gyazo.com/284db4c3a1fc0ee5599e6a8e8b3fd2dc.gif

mattermod · 2020-04-20T16:57:55Z

Test server creation failed. See the logs for more information.

asaadmahmood · 2020-04-20T17:01:43Z

@esethna I feel like we can make it not clickable. I don't want people to accidentally click it and lose the RHS, as it does not close it right now.
In the case of the user opening the reply thread from a post in the center channel, he would always remain in that channel, as one can't easily navigate to a different channel with the RHS open, so I think its fine.

esethna · 2020-04-20T17:05:36Z

@matthewbirtch @asaadmahmood 0/5, but if we are adding a max width to the RHS expanded state, which is a great idea, do we need the channel sidebar to have an overlay? I'm imagining that if a user switches channels we can just switch the center pane and shrink the RHS back to normal size?

asaadmahmood · 2020-04-20T17:14:13Z

@esethna Removed the click on mobile.
And for the RHS overlay. It's a good way to collapse it right now. If we remove that, then the user would have to manually collapse it, or redirect himself to a different channel to do so.

Plus, if we are removing the overlay, then I would imagine we should also squish the center channel based on the space available, so the user can interact with it, but that may pose challenges on most normal browser screen as the width would be quite small.

esethna · 2020-04-20T17:14:55Z

@asaadmahmood I'm referring to removing the overlay on the channel sidebar, not the center pane. If we do that I don't think we need to squish the center pane

asaadmahmood · 2020-04-20T17:15:49Z

@esethna Hmmm, yeah I think we can try that.

asaadmahmood · 2020-04-20T17:17:49Z

Or maybe it may not look as good. It would look like this.

asaadmahmood · 2020-04-20T17:34:47Z

With the overlay, its clear that clicking the sidebar would collapse it, but without it, its not as clear.

asaadmahmood · 2020-04-20T17:41:00Z

Also, having the sidebar collapse on channel switch might be something we can consider for a different ticket, as that would be adding some extra functionality on a global level, so the channel sidebar can close it.

hmhealey · 2020-04-20T17:46:29Z

utils/utils.jsx

- const ownPostBg = blendColors(theme.centerChannelBg, theme.centerChannelColor, 0.05);
- const hoveredPostBg = blendColors(theme.centerChannelBg, theme.centerChannelColor, 0.08);
- const hoveredPostBgLight = blendColors(theme.centerChannelBg, theme.centerChannelColor, 0.05);
+ const ownPostBg = theme.centerChannelBg;


Instead of just changing the colour here, could you remove the logic that refers to ownPostBg? This part of the code is really complicated, so removing that case would simplify it a fair bit.

andrewbrown00 · 2020-04-20T20:45:39Z

@asaadmahmood things look great here 🎉

A few items to resolve/consider:

We should use the permalink highlight pattern after a user clicks on the channel name in the Thread header. Right now, clicking the channel name in the Thread header opens the channel (unless it's already open); but we don't provide the user with an affordance to where the thread is in the channel.
0/5 and open to feedback, but something feels off with the timing when clicking to expand the RHS. I think it's because the overlay comes in faster than the RHS slides out. I like the animation on the RHS but wonder if we can tie them together?
0/5 but something feels off with leaving a sliver of the center channel showing when the RHS is expanded; it's create a stepped/staggered visual. Maybe it won't appear this way once the overlay is removed from the LHS.
@esethna should we add tooltips to the expand and close icons. We need to define the rules for when we do/do not want to introduce tooltips for icon buttons.
The overlay sits on top of the Thread view when reducing the view down to mobile.

esethna · 2020-04-20T22:46:56Z

@andrewbrown00

That's a really great idea to permalink to the root post in the center pane - looks like that's what slack does also. We should do this for sure.

Re tooltips, already have a ticket completed for that by community: https://mattermost.atlassian.net/browse/MM-22876

@asaadmahmood I don't mind this but I'm 0/5, I've leave it up to you guys and @andrewbrown00 on what's the best UI for the expanded state. Wonder if we can find some examples in other products

mm-cloud-bot · 2020-04-30T18:39:01Z

New commit detected. SpinWick will upgrade if the updated docker image is available.

mm-cloud-bot · 2020-04-30T18:53:00Z

Mattermost test server updated with git commit 15806433e2e6291c798e0a0a572bc0299f3c4205.

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

lindy65 · 2020-05-01T09:03:24Z

Thanks @asaadmahmood - pshew, this is a hectic PR to review - so many comments! ;)

I tested via the Jira tickets linked at the top of this PR, our DM where you gave me the high level changes and also from design and PM comments above. Hope I did not miss anything!

1. In RHS thread view, the "<" (back arrow) is misaligned with "Thread". This is visible after a search is performed and "reply" is hit on one of the search results.

2. In RHS thread view, where the channel name is clickable:
- This doesn't seem very "obvious" to me as a user (there is a grey hover state and mouse pointer changes to a hand but just looking at the channel name doesn't say to me "hey, you can click here to open the channel in the center pane"). Is it possible to show the channel name in a "link colour" (e.g. blue) so it looks like a link you can click on (both on hover and without hover over)? Similar to the "Help" link below "Add a comment" in this screenshot...

3. In expanded RHS view:
- I do not see a drop shadow on the LHS bar?
- The top left corner is not rounded (but not sure whether this was implemented as it has a question mark next to it on the Jira ticket
- ESC is not working (to close the expanded RHS)

asaadmahmood · 2020-05-01T09:31:00Z

@lindy65

Fixed.
I agree, however, with the pattern we are developing, with the icons in the header, or the post menu icons, it is consistent. Gray, gray and background on hover, and then button bg on click and hover. So we'll probably keep it that way, and maybe add an icon next to the name in the future.

There isn't supposed to be, we removed it.
We went with not rounding it off.
Yeah, that was not done in this PR.

mm-cloud-bot · 2020-05-01T09:32:09Z

New commit detected. SpinWick will upgrade if the updated docker image is available.

lindy65

Thanks @asaadmahmood - sounds good to me 👍

Please help update the Jira tickets linked to this PR to reflect changes that actually made it in to the PR? Thanks!

mm-cloud-bot · 2020-05-01T09:42:07Z

Test server destroyed

mm-cloud-bot · 2020-05-01T09:47:42Z

Test server creation failed. See the logs for more information.

asaadmahmood added 1: PM Review Requires review by a product manager 1: UX Review Requires review by a UX Designer labels Apr 19, 2020

asaadmahmood added this to the v5.24.0 milestone Apr 19, 2020

asaadmahmood requested review from matthewbirtch and esethna April 19, 2020 09:58

esethna added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Apr 19, 2020

matthewbirtch suggested changes Apr 20, 2020

View reviewed changes

hmhealey suggested changes Apr 20, 2020

View reviewed changes

hmhealey changed the title ~~Quick wins bucket list 5~~ MM-22894/MM-23046/MM-24375 Quick wins bucket list 5 Apr 20, 2020

asaadmahmood added 6 commits April 30, 2020 23:37

Updating icons

b5b45c3

Updating icons

e017093

Updating minor code

b4b90a7

Updating minor functional changes

2123afb

Updating translation

00cc5d4

Updating channel state

1580643

hmhealey approved these changes Apr 30, 2020

View reviewed changes

hmhealey removed the 2: Dev Review Requires review by a core commiter label Apr 30, 2020

Updating back button

59ee6fc

lindy65 approved these changes May 1, 2020

View reviewed changes

lindy65 added 4: Reviews Complete All reviewers have approved the pull request QA Review Done and removed 3: QA Review Requires review by a QA tester Setup Cloud Test Server Setup a test server using Mattermost Cloud labels May 1, 2020

asaadmahmood added the Do Not Merge Should not be merged until this label is removed label May 1, 2020

Updating font icons

41c03e7

asaadmahmood removed the Do Not Merge Should not be merged until this label is removed label May 1, 2020

asaadmahmood merged commit ba7358a into mattermost:master May 1, 2020

asaadmahmood deleted the quick-wins-5 branch May 1, 2020 17:15

abdusabri mentioned this pull request May 11, 2020

MM-24965 Fix issue of search results stuck loading in mobile web view #5461

Closed

agarciamontoro mentioned this pull request May 13, 2020

MM-24965: Re-evaluate transitions on RHS state change #5483

Merged

amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels May 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MM-22894/MM-23046/MM-24274/MM-24375 Quick wins bucket list 5 #5353

MM-22894/MM-23046/MM-24274/MM-24375 Quick wins bucket list 5 #5353

asaadmahmood commented Apr 19, 2020 •

edited by hmhealey

Loading

mattermod commented Apr 19, 2020

mattermod commented Apr 19, 2020

esethna commented Apr 19, 2020 •

edited

Loading

asaadmahmood commented Apr 19, 2020 •

edited

Loading

mattermod commented Apr 19, 2020

matthewbirtch left a comment

asaadmahmood commented Apr 20, 2020

asaadmahmood commented Apr 20, 2020 •

edited

Loading

matthewbirtch commented Apr 20, 2020

asaadmahmood commented Apr 20, 2020

esethna commented Apr 20, 2020 •

edited

Loading

mattermod commented Apr 20, 2020

asaadmahmood commented Apr 20, 2020

esethna commented Apr 20, 2020 •

edited

Loading

asaadmahmood commented Apr 20, 2020

esethna commented Apr 20, 2020 •

edited

Loading

asaadmahmood commented Apr 20, 2020

asaadmahmood commented Apr 20, 2020 •

edited

Loading

asaadmahmood commented Apr 20, 2020

asaadmahmood commented Apr 20, 2020

hmhealey Apr 20, 2020

andrewbrown00 commented Apr 20, 2020 •

edited by asaadmahmood

Loading

esethna commented Apr 20, 2020

mm-cloud-bot commented Apr 30, 2020

mm-cloud-bot commented Apr 30, 2020

lindy65 commented May 1, 2020

asaadmahmood commented May 1, 2020

mm-cloud-bot commented May 1, 2020

lindy65 left a comment

mm-cloud-bot commented May 1, 2020

mm-cloud-bot commented May 1, 2020

MM-22894/MM-23046/MM-24274/MM-24375 Quick wins bucket list 5 #5353

MM-22894/MM-23046/MM-24274/MM-24375 Quick wins bucket list 5 #5353

Conversation

asaadmahmood commented Apr 19, 2020 • edited by hmhealey Loading

Summary

Ticket Link

Screenshots

mattermod commented Apr 19, 2020

mattermod commented Apr 19, 2020

esethna commented Apr 19, 2020 • edited Loading

asaadmahmood commented Apr 19, 2020 • edited Loading

mattermod commented Apr 19, 2020

matthewbirtch left a comment

Choose a reason for hiding this comment

asaadmahmood commented Apr 20, 2020

asaadmahmood commented Apr 20, 2020 • edited Loading

matthewbirtch commented Apr 20, 2020

asaadmahmood commented Apr 20, 2020

esethna commented Apr 20, 2020 • edited Loading

mattermod commented Apr 20, 2020

asaadmahmood commented Apr 20, 2020

esethna commented Apr 20, 2020 • edited Loading

asaadmahmood commented Apr 20, 2020

esethna commented Apr 20, 2020 • edited Loading

asaadmahmood commented Apr 20, 2020

asaadmahmood commented Apr 20, 2020 • edited Loading

asaadmahmood commented Apr 20, 2020

asaadmahmood commented Apr 20, 2020

hmhealey Apr 20, 2020

Choose a reason for hiding this comment

andrewbrown00 commented Apr 20, 2020 • edited by asaadmahmood Loading

esethna commented Apr 20, 2020

mm-cloud-bot commented Apr 30, 2020

mm-cloud-bot commented Apr 30, 2020

lindy65 commented May 1, 2020

asaadmahmood commented May 1, 2020

mm-cloud-bot commented May 1, 2020

lindy65 left a comment

Choose a reason for hiding this comment

mm-cloud-bot commented May 1, 2020

mm-cloud-bot commented May 1, 2020

asaadmahmood commented Apr 19, 2020 •

edited by hmhealey

Loading

esethna commented Apr 19, 2020 •

edited

Loading

asaadmahmood commented Apr 19, 2020 •

edited

Loading

asaadmahmood commented Apr 20, 2020 •

edited

Loading

esethna commented Apr 20, 2020 •

edited

Loading

esethna commented Apr 20, 2020 •

edited

Loading

esethna commented Apr 20, 2020 •

edited

Loading

asaadmahmood commented Apr 20, 2020 •

edited

Loading

andrewbrown00 commented Apr 20, 2020 •

edited by asaadmahmood

Loading