fix(datepicker): calendar panel theme supports dark mode #11201

rudzikdawid · 2018-03-29T20:37:57Z

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 datepicker calendar panel is always using light mode hues even when the theme is dark mode.

Issue Number:
#11200

What is the new behavior?

The datepicker calendar 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

Before dark mode:

After dark mode:

After light mode:

googlebot · 2018-03-29T20:38:00Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

rudzikdawid · 2018-03-29T20:44:14Z

I signed it!

googlebot · 2018-03-29T20:44:16Z

CLAs look good, thanks!

Splaktar · 2018-03-30T01:43:06Z

Thank you for your contribution! Update the commit message to include Closes #11200 on the last line, after a blank line. I'll try to take a look at the color changes and verify. After looking at the changes, there is one color that I am a little concerned about, but want to test.

rudzikdawid · 2018-03-30T07:30:51Z

ok commit message fixed.
I was thinking about

tr:last-child td {
     border-bottom-color: '{{background-200}}';
}

but any value rgba with alpha less than 1: {{background-200-0.2}} makes ugly border.
border-collapse: collapse makes the lines overlap each others. With rgba alpha=1 it's not a problem but with a little bit transparent lines it is big glitch.

that's why i did not change it

Splaktar · 2018-04-05T02:38:46Z

Please make sure to test effects of your changes in light mode as well. As you can see below, the focused background is much lighter and the day of week background is missing/white.

Before:

After:

Splaktar

Please address the changes to light mode (which ideally should be kept the same, or only very slightly modified if the modification is specified by the spec).

rudzikdawid · 2018-04-05T10:55:17Z

https://material.io/guidelines/components/pickers.html#pickers-date-pickers
in official material style guide there is no difference between months header background and calendar background. There is only differences between color of the font.

also in material2 implementation there is no difference between these background colors
https://material.angular.io/components/datepicker/overview

Why in material1 You want keep these background color difference ?
Maybe we should just change the color of the fonts in the months header ?

rudzikdawid · 2018-04-05T20:56:04Z

ok i pushed fix
hover background: '{{background-500-0.33}}'; looks almost the same in light mode as master build
but also its ok i dark mode

Also i changed separeted lines

tr:last-child td {
  border-bottom-color: '{{background-hue-2}}';
}

before:

after:

but still in my opinion we should change background color of months header to the calendar background color. There should be no differences between these background colors. But we can open another issue for that

Splaktar · 2018-04-07T08:27:44Z

Yes, I think that months header change should be part of a separate issue/PR so that we don't put these changes at additional risk. Thanks for updating. I'll do some manually testing on this soon.

Splaktar

Thank you for putting this together. There appear to be some small differences in light mode themes which I've made notes about.

Splaktar · 2018-04-08T06:09:25Z

src/components/datepicker/calendar-theme.scss

@@ -28,7 +28,7 @@
  .md-calendar-date-selection-indicator {
    .md-calendar-date.md-focus &,
    &:hover {
-      background: '{{background-300}}';
+      background: '{{background-500-0.33}}';


This seems very slightly off in Light mode.

master => rgb(224, 224, 224).

This branch => rgba(158, 158, 158, 0.33) which converts to rgb(223, 223, 223).

'{{background-500-0.32}}' its better => rgb(224, 224, 224)

Splaktar · 2018-04-08T06:12:50Z

src/components/datepicker/calendar-theme.scss

  }
 }
 .md-THEME_NAME-theme {
  .md-calendar-day-header {
-    background: '{{background-300}}';
-    color: '{{background-A200-0.87}}';
+    background: '{{background-500-0.33}}';


Same as below with being just slightly different than master in light mode.

should be'{{background-500-0.32}}'

Splaktar · 2018-04-08T06:16:17Z

src/components/datepicker/calendar-theme.scss


  tr:last-child td {
-    border-bottom-color: '{{background-200}}';
+    border-bottom-color: '{{background-hue-2}}';


Light mode:

master => rgb(238, 238, 238)

updates => rgb(245, 245, 245)

i can't find proper value with alpha channel === 1.0 which looks good in dark mode and also looks the same as master build. Using alpha channel less than 1.0 makes render glitches.
I think '{{background-hue-2}}' is a reasonable compromise. We have very small difference in light mode whitch doesnt' affect on UX and also good dark theme mode.

OK, we can try to presubmit it using that value and see if it gets through.

Splaktar · 2018-04-08T06:21:21Z

src/components/datepicker/calendar-theme.scss

-  background: '{{background-A100}}';
-  color: '{{background-A200-0.87}}';
+  background: '{{background-hue-1}}';
+  color: '{{foreground-1}}';


Can you please add a simple md-calendar demo?
We have the directive docs but there is no demo for using the md-calendar directly w/o an md-datepicker.

md-calendar is name of whole overlapping panel, check DOM tree

<table role="grid" tabindex="0" class="md-calendar" aria-readonly="true">

also you can enter into special calendar mode by open datepicker and click into .md-calendar-month-label

Right, but this is targeting class="md-calendar" via .md-calendar and not the md-calendar element via CSS. md-datepicker doesn't seem to apply the md-calendar class at all.

But just using md-calendar directly seems to apply this class to the table:

md-datepicker doesn't seem to apply the md-calendar class at all.

not exactly. Simple test shows something different:

.md-calendar.md-THEME_NAME-theme { background: red; }

after that whole overlapping panel has red background. So md-datepicker apply .md-calendar class to panel

Interesting. I wasn't able to find the class in the DOM, but I'll take your word for it.

Oh I found it now. Strange that Chrome DevTools search didn't work...

rudzikdawid · 2018-04-08T08:43:24Z

done i pushed fixes

After dark mode:

After light mode:

Splaktar · 2018-04-09T07:25:41Z

src/components/datepicker/calendar-theme.scss

@@ -28,7 +28,7 @@
  .md-calendar-date-selection-indicator {
    .md-calendar-date.md-focus &,
    &:hover {
-      background: '{{background-300}}';
+      background: '{{background-500-0.32}}';
    }
  }



The comments on Lines 47-50 need to be updated as they are no longer accurate. Perhaps just removing ", even on the dark theme," is enough.

.md-calendar-date-disabled, .md-calendar-month-label-disabled {

I think that we need to update the disabled date/month colors too.

You have right https://material.angularjs.org/latest/demo/datepicker
example Only weekends are selectable

i will fix it

what do You think about

.md-calendar-date-disabled, .md-calendar-month-label-disabled { color: '{{foreground-3}}'; }

its a litle bit brighter than background-A200-0.435 in light mode.
But in both modes light/dark looks great.

a lot of components use '{{foreground-3}}' in [disabled] selectors.
So its seems right. What do you think? Can we use it?

example '{{foreground-3}}'

Sounds good. There is a comment in theming.js:
* {{foreground-3}} - used for disabled text.

fix was pushed

Splaktar · 2018-04-09T08:17:01Z

If I use the docs-dark theme with this branch, I'm seeing the following

If I make a modification to calendar-theme.scss line 3 to:

.md-calendar.md-THEME_NAME-theme,
.md-THEME_NAME-theme .md-calendar {
  background: '{{background-hue-1}}';
  color: '{{foreground-1-0.87}}';

Then I get this:

Note the black chevron icons and white popup border.

I'm not sure why I am seeing such a difference from the screenshots that you posted.

rudzikdawid · 2018-04-09T08:20:52Z

i used

$mdThemingProvider.theme('default')
      .primaryPalette('docs-blue')
      .accentPalette('docs-red')
      .dark();

i dont see dark icons in dark theme. Also with this config i have bright icons:

$mdThemingProvider.theme('default')
      .primaryPalette('yellow')
      .dark();

Closes angular#11200

Splaktar · 2018-04-09T10:23:24Z

OK, yeah, changing the theme works best in docs/app/js/app.js rather than in the examples themselves.

Splaktar

LGTM

Closes angular#11200

Closes #11200

googlebot added the cla: no PR author needs to sign Google's CLA: https://opensource.google.com/docs/cla/ label Mar 29, 2018

googlebot added cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ and removed cla: no PR author needs to sign Google's CLA: https://opensource.google.com/docs/cla/ labels Mar 29, 2018

rudzikdawid mentioned this pull request Mar 29, 2018

select: dropdown doesn't inherit dark theme #3379

Closed

Splaktar self-assigned this Mar 30, 2018

Splaktar requested review from crisbeto and Splaktar March 30, 2018 01:39

Splaktar added this to the 1.1.9 milestone Mar 30, 2018

Splaktar added the needs: manual testing This issue or PR needs to have some manual testing and verification done label Mar 30, 2018

Splaktar added type: bug ui: theme P4: minor Minor issues. May not be fixed without community contributions. labels Mar 30, 2018

rudzikdawid force-pushed the fixDatepickerPopupTheme branch from f034a52 to 0e927f1 Compare March 30, 2018 07:19

Splaktar suggested changes Apr 5, 2018

View reviewed changes

Splaktar added in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs and removed needs: manual testing This issue or PR needs to have some manual testing and verification done labels Apr 5, 2018

rudzikdawid force-pushed the fixDatepickerPopupTheme branch from 0e927f1 to 808b22f Compare April 5, 2018 20:47

Splaktar added the needs: manual testing This issue or PR needs to have some manual testing and verification done label Apr 7, 2018

Splaktar suggested changes Apr 8, 2018

View reviewed changes

Splaktar removed the needs: manual testing This issue or PR needs to have some manual testing and verification done label Apr 8, 2018

rudzikdawid force-pushed the fixDatepickerPopupTheme branch from 808b22f to 1698fb9 Compare April 8, 2018 08:41

Splaktar added the needs: manual testing This issue or PR needs to have some manual testing and verification done label Apr 8, 2018

Splaktar reviewed Apr 9, 2018

View reviewed changes

Splaktar removed the needs: manual testing This issue or PR needs to have some manual testing and verification done label Apr 9, 2018

fix(datepicker): calendar panel theme supports dark mode

b8ffb2a

Closes angular#11200

rudzikdawid force-pushed the fixDatepickerPopupTheme branch from 1698fb9 to b8ffb2a Compare April 9, 2018 08:31

Splaktar approved these changes Apr 9, 2018

View reviewed changes

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 9, 2018

Splaktar assigned jelbourn and unassigned Splaktar Apr 9, 2018

jelbourn assigned tinayuangao and unassigned jelbourn Apr 9, 2018

tinayuangao merged commit 2360764 into angular:master Apr 10, 2018

chmelevskij pushed a commit to chmelevskij/material that referenced this pull request Jun 19, 2018

fix(datepicker): calendar panel theme supports dark mode (angular#11201)

aaf9ef4

Closes angular#11200

Splaktar pushed a commit that referenced this pull request Jul 31, 2018

fix(datepicker): calendar panel theme supports dark mode (#11201)

dd7f352

Closes #11200

fix(datepicker): calendar panel theme supports dark mode #11201

fix(datepicker): calendar panel theme supports dark mode #11201

Conversation

rudzikdawid commented Mar 29, 2018 • edited Loading

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

googlebot commented Mar 29, 2018

What to do if you already signed the CLA

Individual signers

Corporate signers

rudzikdawid commented Mar 29, 2018 • edited Loading

googlebot commented Mar 29, 2018

Splaktar commented Mar 30, 2018

rudzikdawid commented Mar 30, 2018 • edited Loading

Splaktar commented Apr 5, 2018

Splaktar left a comment

Choose a reason for hiding this comment

rudzikdawid commented Apr 5, 2018 • edited Loading

rudzikdawid commented Apr 5, 2018 • edited Loading

Splaktar commented Apr 7, 2018

Splaktar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rudzikdawid Apr 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rudzikdawid Apr 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rudzikdawid Apr 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rudzikdawid commented Apr 8, 2018 • edited Loading

Choose a reason for hiding this comment

Splaktar Apr 9, 2018 • edited Loading

Choose a reason for hiding this comment

rudzikdawid Apr 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rudzikdawid Apr 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Splaktar commented Apr 9, 2018

rudzikdawid commented Apr 9, 2018 • edited Loading

Splaktar commented Apr 9, 2018

Splaktar left a comment

Choose a reason for hiding this comment

rudzikdawid commented Mar 29, 2018 •

edited

Loading

rudzikdawid commented Mar 29, 2018 •

edited

Loading

rudzikdawid commented Mar 30, 2018 •

edited

Loading

rudzikdawid commented Apr 5, 2018 •

edited

Loading

rudzikdawid commented Apr 5, 2018 •

edited

Loading

rudzikdawid Apr 8, 2018 •

edited

Loading

rudzikdawid Apr 8, 2018 •

edited

Loading

rudzikdawid Apr 8, 2018 •

edited

Loading

rudzikdawid commented Apr 8, 2018 •

edited

Loading

Splaktar Apr 9, 2018 •

edited

Loading

rudzikdawid Apr 9, 2018 •

edited

Loading

rudzikdawid Apr 9, 2018 •

edited

Loading

rudzikdawid commented Apr 9, 2018 •

edited

Loading