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

Fix plugin visuals to handle different themes #116

Merged
merged 6 commits into from
Sep 2, 2020

Conversation

colorfusion
Copy link
Contributor

@colorfusion colorfusion commented Aug 27, 2020

Summary

This pull request will adjust the visuals of the plugin, allowing the plugin to adapt to different themes in the application accordingly.
The style has been adjusted slightly to differentiate the different components in the plugin, and to follow Mattermost's layout more closely.

Ticket Link

Fixes #78

Screenshot

Sidebar

image
image

Add / Request Todo (root page)

image image
image image

Todo Message Input

image
image

@colorfusion colorfusion requested a review from larkox as a code owner August 27, 2020 04:26
@codecov-commenter
Copy link

codecov-commenter commented Aug 27, 2020

Codecov Report

Merging #116 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #116   +/-   ##
======================================
  Coverage    3.97%   3.97%           
======================================
  Files           9       9           
  Lines         930     930           
======================================
  Hits           37      37           
  Misses        893     893           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 76cd654...3d25e9d. Read the comment docs.

@larkox larkox requested a review from asaadmahmood August 27, 2020 08:10
@larkox larkox added 1: UX Review Requires review by a UX Designer 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Aug 27, 2020
@larkox larkox requested a review from hanzei August 27, 2020 08:11
Copy link
Contributor

@larkox larkox left a comment

Choose a reason for hiding this comment

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

Awesome job, @colorfusion ! Thank you very much.

LGMT

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.

@colorfusion We only use theme variables in the multiples of 4.

@asaadmahmood
Copy link
Contributor

@colorfusion We only use theme variables in the multiples of 4.

You can see the variables available in the utils file of the webapp:
https://github.com/mattermost/mattermost-webapp/blob/master/utils/utils.jsx
Screenshot 2020-08-27 at 1 12 08 PM

@colorfusion
Copy link
Contributor Author

I've adjusted the theme variables to be in multiple of 4, and fixed the input box's border with new screenshots in the PR.
Do let me know if there are any additional changes that should be made as well.

Copy link
Contributor

@hanzei hanzei left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@hanzei hanzei removed the 1: UX Review Requires review by a UX Designer label Aug 28, 2020
Copy link
Contributor

@larkox larkox left a comment

Choose a reason for hiding this comment

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

LGTM!

@larkox larkox removed the 2: Dev Review Requires review by a core committer label Aug 31, 2020
@larkox larkox requested a review from DHaussermann August 31, 2020 10:04
Copy link

@DHaussermann DHaussermann left a comment

Choose a reason for hiding this comment

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

Tested and passed

  • Tabs now look like tabs and not buttons
  • Typeface color on unselected tab now has more contrast on darker themes to be more readable
  • Tested all themes
  • Color scheme of To Do seems consistent with other UI elements when theme is applied
  • No issues found
    LGTM!

Huge thanks @colorfusion for implementing this! Your changes have hugely improved the UI for this plugin. Much appreciated.

@DHaussermann DHaussermann added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Sep 1, 2020
@larkox larkox merged commit 6cce8b3 into mattermost-community:master Sep 2, 2020
@colorfusion colorfusion deleted the feature/fix-theme branch September 2, 2020 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve look on different themes
6 participants