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

fix(theme): prepend # to hex codes enableBrowserColor #11492

Merged
merged 1 commit into from
Nov 6, 2018

Conversation

jagmeetb
Copy link
Contributor

@jagmeetb jagmeetb commented Oct 22, 2018

PR Checklist

Please check that your PR fulfills the following requirements:

  • The commit message follows our guidelines
  • Tests for the changes have been added or this is not a bug fix / enhancement
  • Docs have been added, updated, or were not required

PR Type

What kind of change does this PR introduce?

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

What is the current behavior?

Issue Number: fixes #11259

enableBrowserColor fails silently if a hex code does not have a # at the start.

What is the new behavior?

enableBrowserColor now checks for a leading # and prepends it if not already there.

Does this PR introduce a breaking change?

[ ] Yes
[X] No

Other information

I updated other test plans to expect the leading #, the testPalette in the test plans did not have a # in their hex values. Originally I tried updating the palette to have a #, but couldn't figure out how to test it without creating another palette, so I went with this way.

@googlebot googlebot added the cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ label Oct 22, 2018
@jagmeetb
Copy link
Contributor Author

jagmeetb commented Oct 23, 2018

I am able to get all tests to pass on my local machine

Chrome 70.0.3538 (Windows 7.0.0): Executed 2884 of 2885 (skipped 1) SUCCESS (43.351 secs / 42.579 secs)
Firefox 62.0.0 (Windows 7.0.0): Executed 2884 of 2885 (skipped 1) SUCCESS (1 min 15.55 secs / 1 min 11.396 secs)

Googling the error code on the Travis tests suggests that it is something to do with Karma1.6, but I'm still unsure.

Please let me know if there's something I need to change.

edit: squashed my commits and the version1.6 tests now succeeds but the 1.7 fails due to timeout. Would this be a fluke?

check for leading # and prepend
update enableBrowserColor tests

Fixes angular#11259
@Splaktar Splaktar self-requested a review October 23, 2018 18:54
@Splaktar Splaktar self-assigned this Oct 23, 2018
@Splaktar
Copy link
Contributor

I've restarted the tests, but we are waiting on PR #11488 to get merged which should help fix some test issues on master.

@Splaktar Splaktar added ui: theme type: bug needs: review This PR is waiting on review from the team P4: minor Minor issues. May not be fixed without community contributions. labels Oct 23, 2018
@Splaktar Splaktar modified the milestones: 1.1.11, 1.1.12 Oct 23, 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.

LGTM

@Splaktar Splaktar added pr: merge ready This PR is ready for a caretaker to review and removed needs: review This PR is waiting on review from the team labels Oct 23, 2018
@Splaktar Splaktar modified the milestones: 1.1.12, 1.1.11 Oct 23, 2018
@Splaktar Splaktar assigned mmalerba and jelbourn and unassigned mmalerba Oct 23, 2018
@jelbourn jelbourn merged commit 0306ac0 into angular:master Nov 6, 2018
marosoft pushed a commit to marosoft/material that referenced this pull request Nov 11, 2018
check for leading # and prepend
update enableBrowserColor tests

Fixes angular#11259
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: 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.

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