Material: theme: enableBrowserColor fails silently if HEX color doesn't have a # at the start

[mymattcarroll]

Bug:

$mdThemingProvider.enableBrowserColor() does not work when using a custom palette

What is the expected behavior?

Browser colour should change to the custom palette colour

What is the current behavior?

Browser does not have colour at all.

CodePen and steps to reproduce the issue:

CodePen Demo which shows your issue:

Only works on a Mobile so here is some code:

$mdThemingProvider.definePalette('myorange', {
  '50': 'fcf2e0',
  '100': 'f7deb3',
  '200': 'f2c880',
  '300': 'edb14d',
  '400': 'e9a126',
  '500': 'e59000',
  '600': 'e28800',
  '700': 'de7d00',
  '800': 'da7300',
  '900': 'd36100',
  'A100': 'fffcfb',
  'A200': 'ffdec8',
  'A400': 'ffbf95',
  'A700': 'ffaf7b',
  'contrastDefaultColor': 'light',
  'contrastDarkColors': [
    '50',
    '100',
    '200',
    '300',
    '400',
    'A100',
    'A200'
  ],
  'contrastLightColors': [
    '500',
    '600',
    '700',
    '800',
    '900',
    'A400',
    'A700'
  ]
})
$mdThemingProvider.definePalette('mygrey', {
  '50': 'e5e6e6',
  '100': 'bec0c1',
  '200': '929698',
  '300': '666b6e',
  '400': '464c4f',
  '500': '252c30',
  '600': '21272b',
  '700': '1b2124',
  '800': '161b1e',
  '900': '0d1013',
  'A100': '58acff',
  'A200': '2592ff',
  'A400': '0079f1',
  'A700': '006cd8',
  'contrastDefaultColor': 'light',
  'contrastDarkColors': [
    '50',
    '100',
    'A100'
  ],
  'contrastLightColors': [
    '200',
    '300',
    '400',
    '500',
    '600',
    '700',
    '800',
    '900',
    'A200',
    'A400',
    'A700'
  ]
})

$mdThemingProvider.theme('default')
  .primaryPalette('myorange')
  .accentPalette('mygrey')
$mdThemingProvider.enableBrowserColor({
  palette: 'myorange'
})

Detailed Reproduction Steps:

See above code snippet.

What is the use-case or motivation for changing an existing behavior?

I want to be able to use my custom palette to change mobile browsers

Which versions of AngularJS, Material, OS, and browsers are affected?

• AngularJS: 1.6.9
• AngularJS Material: 1.1.8
• OS: Android 7
• Mobile Browsers:
  
  
  
  • Samsung Internet
  
  
  • Google Chrome
  
  

Is there anything else we should know? Stack Traces, Screenshots, etc.

n/a

[Splaktar]

A CodePen demo if this would still be helpful for verification and debugging. We can load that example on mobile just fine, if provided.

[Splaktar]

Created a CodePen demo from the code snippet provided.
Also created a CodePen demo of using a built in theme.

From the docs:

palette - Can be any one of the basic material design palettes, extended defined palettes and 'primary', 'accent', 'background' and 'warn'. Default is primary.

[Splaktar]

OK, here's a working CodePen using your custom palette (except for hue 800). As mentioned in the docs, you have to extend an existing palette for this feature to work.

The hue that you use for the Browser color has to be one of the built in hues. So by default, you can skip overriding 800 and then it will work (commented out in the CodePen). Another option is to choose a different hue (that you won't override).

[mymattcarroll]

@Splaktar , thank you heaps for making the CodePen.

Ill attempt to get this working tomorrow by extending an existing palette and let you know how I go.

[mymattcarroll]

@Splaktar , after some debugging it turns out I can use the timorange palette I have defined. It just does not work if the colors defined do not have a # in front of the hex value. e.g. changing to this has made it work perfectly:

$mdThemingProvider.definePalette('timorange', {
  '50': '#fcf2e0',
  '100': '#f7deb3',
  '200': '#f2c880',
  '300': '#edb14d',
  '400': '#e9a126',
  '500': '#e59000',
  '600': '#e28800',
  '700': '#de7d00',
  '800': '#da7300',
  '900': '#d36100',
  'A100': '#fffcfb',
  'A200': '#ffdec8',
  'A400': '#ffbf95',
  'A700': '#ffaf7b',
  'contrastDefaultColor': 'light',
  'contrastDarkColors': [
    '50',
    '100',
    '200',
    '300',
    '400',
    'A100',
    'A200'
  ],
  'contrastLightColors': [
    '500',
    '600',
    '700',
    '800',
    '900',
    'A400',
    'A700'
  ]
})
$mdThemingProvider.definePalette('timgrey', {
  '50': '#e5e6e6',
  '100': '#bec0c1',
  '200': '#929698',
  '300': '#666b6e',
  '400': '#464c4f',
  '500': '#252c30',
  '600': '#21272b',
  '700': '#1b2124',
  '800': '#161b1e',
  '900': '#0d1013',
  'A100': '#58acff',
  'A200': '#2592ff',
  'A400': '#0079f1',
  'A700': '#006cd8',
  'contrastDefaultColor': 'light',
  'contrastDarkColors': [
    '50',
    '100',
    'A100'
  ],
  'contrastLightColors': [
    '200',
    '300',
    '400',
    '500',
    '600',
    '700',
    '800',
    '900',
    'A200',
    'A400',
    'A700'
  ]
})

$mdThemingProvider.theme('default')
  .primaryPalette('timorange')
  .accentPalette('timgrey', {
    default: '500'
  })
$mdThemingProvider.enableBrowserColor({
  palette: 'timorange'
})

Should this line be updated to add a # as a prefix to the color variable to ensure valid custom palettes work. I would be happy to submit a Pull Request if this change seems appropriate.

[Splaktar]

It should probably check if there is a # and if not, one should be prepended.

[sridhar-shaw]

Is this still open...? If so can I pick this ?

[Splaktar]

@sridhar-shaw yes this is still open. There is no PR open to fix this, so yes we would appreciate if you could start working on a PR.

Sorry for the delay, I just returned from a couple conferences.

[jagmeetb]

Hello, I'm interested in submitting a PR for this issue. Is anyone working on this currently?

I'm still new to to contributing in open-source, and just to make sure I'm understanding this issue correctly: I need to update this line to prepend a # if none is there?

[Splaktar]

@jagmeetb I don't believe that anyone is working on this atm. There is no PR open. Yes, that appears to be all that is needed along with a unit test to verify that the issue is fixed.

The Contributing Guide has a lot of information on how to format your commit message and other important details to get you started.

Then the Pull Request Guide contains the information that you need for submitting a PR and dealing with review feedback.

You can ask for help in this issue, the AngularJS Material forums, or on the AngularJS Material Gitter.

[jagmeetb]

I've been trying to work on this solution, but the initial change I tried:

var color = angular.isObject(palette[hue]) ? palette[hue].hex : palette[hue];
if (color.substr(0, 1) !== '#') color = '#' + color;    //my line

seems to break some other tests. Those tests have palettes that also exclude the the # sign, and expect the enableBrowserColor method to return hex codes without it.

I don't think editing other tests is the solution, but I'm also not sure how I should proceed without breaking other tests.

[Splaktar]

@jagmeetb sorry for the delay. I was away for the week presenting at AngularMix. In this case, I think that updating those tests is the right way to go.