Skip to content
This repository was archived by the owner on Sep 5, 2024. It is now read-only.

fix(menuBar): md-menu-bar panel theme supports dark mode #11258

Merged
merged 1 commit into from
May 16, 2018

Conversation

rudzikdawid
Copy link
Contributor

@rudzikdawid rudzikdawid commented Apr 25, 2018

Closes #11238

PR Checklist

Please check that your PR fulfills the following requirements:

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

The md-menu-bar panel is partially using light mode hues in dark mode.

Issue Number:
Fixes #11238.

What is the new behavior?

The md-menu-bar panel use dark mode hues when the theme is set to dark mode.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

dark mode before:

dark mode after:

light mode before:

light mode after:

@googlebot googlebot added the cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ label Apr 25, 2018
@Splaktar Splaktar self-requested a review April 26, 2018 03:16
@Splaktar Splaktar added this to the 1.1.10 milestone Apr 26, 2018
@Splaktar Splaktar added ui: theme type: bug P4: minor Minor issues. May not be fixed without community contributions. labels Apr 26, 2018
Copy link
Contributor

@Splaktar Splaktar 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. Just a minor change needed for md-toolbar-filler. This actually helped me look at this feature and figure it out better. I'll be opening a PR to re-add it to the menuBar docs and to add it to the toolbar docs.

md-toolbar-filler {
background-color: '{{primary-color}}';
color: '{{background-A100-0.87}}';
color: '{{foreground-1}}';
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use '{{primary-contrast}}' here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

md-icon {
color: '{{background-A100-0.87}}';
color: '{{foreground-2}}';
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use '{{primary-contrast}}' here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@Splaktar
Copy link
Contributor

Splaktar commented Apr 26, 2018

Currently:
screen shot 2018-04-26 at 1 14 55 am

With '{{primary-contrast}}':
screen shot 2018-04-26 at 1 15 27 am

Opened #11260 with the md-toolbar-filler examples.

@Splaktar Splaktar added the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label Apr 26, 2018
Splaktar added a commit that referenced this pull request Apr 26, 2018
register the icon sets in the docs app
fix strange absolute styling alignment issue

Relates to #11258
@Splaktar Splaktar self-assigned this Apr 26, 2018
andrewseguin pushed a commit that referenced this pull request Apr 26, 2018
register the icon sets in the docs app
fix strange absolute styling alignment issue

Relates to #11258
@rudzikdawid rudzikdawid force-pushed the fixMenuBarTheme branch 2 times, most recently from b389826 to 5ab683d Compare April 26, 2018 19:32
Copy link
Contributor

@Splaktar Splaktar left a comment

Choose a reason for hiding this comment

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

LGTM, thank you very much!

@Splaktar Splaktar added pr: merge ready This PR is ready for a caretaker to review pr: lgtm This PR has been approved by the reviewer and removed in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs labels Apr 27, 2018
@Splaktar Splaktar assigned tinayuangao and unassigned Splaktar May 15, 2018
@tinayuangao tinayuangao merged commit 3a21b89 into angular:master May 16, 2018
Splaktar added a commit that referenced this pull request Jul 31, 2018
register the icon sets in the docs app
fix strange absolute styling alignment issue

Relates to #11258
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ P4: minor Minor issues. May not be fixed without community contributions. pr: lgtm This PR has been approved by the reviewer pr: merge ready This PR is ready for a caretaker to review type: bug ui: theme
Projects
None yet
Development

Successfully merging this pull request may close these issues.

menu-bar: does not support dark mode themes
4 participants