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

Remove deprecated color variables with P2.0 update #123

Open
samthecodingman opened this issue Jan 11, 2017 · 1 comment
Open

Remove deprecated color variables with P2.0 update #123

samthecodingman opened this issue Jan 11, 2017 · 1 comment

Comments

@samthecodingman
Copy link

Description

With the Polymer 2.0 update inbound and breaking changes expected, it might be time to clear out the deprecated --text-primary-color and --default-primary-color values from default-theme.html.

The substitution of --default-primary-color to --primary-color is self-explanatory.

However, --text-primary-color is not so simple. From what I've read it had the meaning "text color over primary colored backgrounds". After it's deprecation, this appears to have been patched in many places as --dark-theme-text-color (defaults to #FFFFFF). However usage like this is confusing as well as demonstrated by #110.

I propose renaming --text-primary-color to --primary-contrast-color and --accent-contrast-color, which would be the foreground color on the primary and accent background colors respectively.

The definition of contrast used here being "strikingly different from something else". I believe this to be clear enough to replace the existing --text-primary-color. For a single alternative name, you could use --contrast-color or --contrast-text-color which would default to --dark-theme-text-color.

The word 'contrast' may not be the ideal fit, but IMO it's meaning is concise enough for widespread use.

Reasoning

  • These variables have been deprecated since at least Dec 2015.
  • Switching to the dark theme would be less troublesome with the core elements.
  • Clarity of meaning regarding 'random' use of --dark-theme-text-color variables
  • Ease of changing foreground color of paper-fab-like accent elements (e.g. dark grey foreground)

Conflicts

These are the changes required to roll out this update. "Critical" changes are breaking changes.

  • CRITICAL: Substitute --text-primary-color in paper-fab.html.
  • Non-functional change: Substitute --default-primary-color with --primary-color in paper-dialog-shared-styles.html
  • CRITICAL: Update mentions of --text-primary-color and --default-primary-color in paper-styles/demo/index.html
  • Appropriately substitute --dark-theme-text-color throughout the PolymerElements catalog with --primary-contrast-color or --accent-contrast-color as required.

Requirements for a P2.0 Migration Tool

As part of any Polymer 2.0 Migration Tool, the following substitutions would be performed if this change is implemented.

  • --default-primary-color --> --primary-color
  • --text-primary-color --> --primary-contrast-color in most use cases and --accent-contrast-color in a select few (such as paper-fab).
@notwaldorf
Copy link
Contributor

Unfortunately, we are trying to be minimally breaking change-y, so this won't happen. We would absolutely accept PRs for elements that are using the old values (the ones you mentioned in Conflicts), but I think just having duplicate values here is not such a horrible deal for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants