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

dropdown action button aligned with action buttons #2029

Merged
merged 3 commits into from
Nov 15, 2018
Merged

dropdown action button aligned with action buttons #2029

merged 3 commits into from
Nov 15, 2018

Conversation

scottleedavis
Copy link
Contributor

@scottleedavis scottleedavis commented Nov 11, 2018

this PR was kicked off at @esethna 's request. :)

Summary

This pull request aligns interaction dropdown select buttons with action buttons, so they appear as one group/list
reminder

Ticket Link

This supports the 'remindbot' work of MM-10580, about aligning the styling of a reminder's buttons with that of slackbot's output.

Checklist

  • Ran make check-style to check for style errors (required for all pull requests)
  • Ran make test to ensure unit and component tests passed
  • Needs to be implemented in mobile (link to PR or User Story) . Maybe?
  • Has UI changes

@jasonblais jasonblais added Setup Old Test Server Triggers the creation of a test server 1: PM Review Requires review by a product manager labels Nov 12, 2018
@jasonblais jasonblais removed the Setup Old Test Server Triggers the creation of a test server label Nov 13, 2018
@mattermost mattermost deleted a comment from mattermod Nov 13, 2018
@mattermost mattermost deleted a comment from mattermod Nov 13, 2018
@mattermost mattermost deleted a comment from mattermod Nov 13, 2018
@jasonblais jasonblais added the Setup Old Test Server Triggers the creation of a test server label Nov 13, 2018
@mattermod
Copy link
Contributor

Setup Test Server label detected. Spinmint test server created if build succeeds (checks pass and no conflicts with base branch).

@mattermod
Copy link
Contributor

Spinmint test server created at: https://i-0ff914542f093313a.test.spinmint.com

Test Admin Account: Username: sysadmin | Password: sysadmin

Test User Account: Username: user-1 | Password: user-1

Instance ID: i-0ff914542f093313a

@jasonblais
Copy link
Contributor

@scottleedavis Overall this looks like a great improvement.

1 - There may now be instances where on narrower screens, the dropdowns and buttons are mixed together as attached below

image

whereas previously they were nicely stacked one after the other

image

But that may be okay, given the benefit we gain on wider screens.

@esethna thoughts on point 1 above?

2 - Will this change need to be applied on mobile as well? Or do we leave mobile as-is, given it's a narrower screen so the menus and buttons will most likely stack vertically anyway.


@asaadmahmood

3 - Can you help review the style changes for message menus below? I think they look great to me.

Before:
image

After:
image

@jasonblais jasonblais added the 2: Dev Review Requires review by a core commiter label Nov 13, 2018
@jasonblais jasonblais self-assigned this Nov 13, 2018
@jasonblais jasonblais removed the 2: Dev Review Requires review by a core commiter label Nov 13, 2018
@esethna
Copy link
Contributor

esethna commented Nov 13, 2018

Thanks @jasonblais, RE 1, given the larger horizontal real-estate on webapp I think this is fine, unless there are use cases that break the horizontal layout?

Re 2: Agree, on React Native I think vertically stacking is fine given the small screen widths

Re 3: Styling looks great, thanks for making those tweaks @scottleedavis.

@esethna esethna removed the request for review from asaadmahmood November 13, 2018 23:12
@jasonblais jasonblais removed their request for review November 13, 2018 23:55
@jasonblais jasonblais removed their assignment Nov 13, 2018
@jasonblais jasonblais added 2: Dev Review Requires review by a core commiter and removed 1: PM Review Requires review by a product manager labels Nov 13, 2018
@mattermod
Copy link
Contributor

Spinmint test server destroyed

@jasonblais jasonblais added 1: PM Review Requires review by a product manager and removed Setup Old Test Server Triggers the creation of a test server 2: Dev Review Requires review by a core commiter labels Nov 13, 2018
Copy link
Contributor

@jasonblais jasonblais left a comment

Choose a reason for hiding this comment

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

Discussed with Eric, changes look good.

@jasonblais jasonblais added 2: Dev Review Requires review by a core commiter and removed 1: PM Review Requires review by a product manager labels Nov 14, 2018
Copy link
Member

@hmhealey hmhealey left a comment

Choose a reason for hiding this comment

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

Looks good to me, but I'm also going to add Asaad since he knows the styling best

Copy link
Contributor

@asaadmahmood asaadmahmood left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@enahum
Copy link
Contributor

enahum commented Nov 15, 2018

I'm going to merge this as it has been approved by 2 devs, 2 PMs and UX

@enahum enahum added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Nov 15, 2018
@enahum enahum merged commit fe96edd into mattermost:master Nov 15, 2018
@enahum
Copy link
Contributor

enahum commented Nov 15, 2018

Thanks for this @scottleedavis

@scottleedavis
Copy link
Contributor Author

Great process. Thank you all. :)

@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Nov 17, 2018
@lindy65 lindy65 added Tests/Not Needed Does not require new release tests and removed 4: Reviews Complete All reviewers have approved the pull request labels Dec 2, 2018
@JtheBAB JtheBAB mentioned this pull request Jan 15, 2019
9 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation Tests/Not Needed Does not require new release tests
Projects
None yet
10 participants