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

fix(css): update global css variables to be used #16003

Merged
merged 17 commits into from Oct 23, 2018

Conversation

@brandyscarney
Copy link
Member

commented Oct 18, 2018

Short description of what this resolves:

Updates all of the global variables to make sure their naming is consistent, their default values are correct, they are used properly by the related components, and remove any that are not used.

Changes proposed in this pull request:

  • removes the non mode-specific global Sass variables
  • updates the md and ios global values so that the default value is the css variable
    with mode-specific fallback values
  • removes any Sass variables not related to theming from the global theme file
  • fixes item so it uses the mode-specific background color that is set by the global
    file

Fixes: #15989, #15559

Breaking Changes

Removed Global CSS Variables

The following global CSS variables have been removed for the reasons listed.

Variable Name Reason
--ion-toolbar-color-inactive Unused
--ion-ripple-background-color Unused / Ripple color is based on component
--ion-header-size Removed in favor of using CSS for h1-h6
--ion-header-step Removed in favor of using CSS for h1-h6

Renamed Global CSS Variables

The following global CSS variables have been renamed for the reasons listed.

Old Variable Name New Variable Name Reason
--ion-toolbar-text-color --ion-toolbar-color Variable is not limited to text color
--ion-toolbar-color-active --ion-toolbar-color-activated Consistency with our component variables
--ion-tabbar-text-color --ion-tab-bar-color Variable is not limited to text color
--ion-tabbar-text-color-active --ion-tab-bar-color-activated Consistency with our component variables
--ion-tabbar-background-color --ion-tab-bar-background Applies to the background property
--ion-tabbar-background-color-focused --ion-tab-bar-background-focused Applies to the background property
--ion-item-background-color --ion-item-background Applies to the background property
--ion-item-background-color-active --ion-item-background-activated Applies to the background property / Consistency with our component variables
--ion-item-text-color --ion-item-color Variable is not limited to text color
--ion-placeholder-text-color --ion-placeholder-color Consistency with other variables

Changed Values

iOS Defaults

Style Old Value New Value
Backdrop Color var(--ion-backdrop-color, #000)
Border Color var(--ion-border-color, #dedede)
Box Shadow Color var(--ion-box-shadow-color, #000)
Overlay Background Color var(--ion-overlay-background-color, #f9f9f9)
Tabbar Background Color var(--ion-tabbar-background-color, #f8f8f8) var(--ion-tab-bar-background, #f8f8f8)
Tabbar Background: Focused var(--ion-tabbar-background-color-focused, get-color-shade(#f8f8f8)) var(--ion-tab-bar-background-focused, get-color-shade(#f8f8f8))
Tabbar Border Color rgba(var(--ion-tabbar-border-color-rgb, 0, 0, 0), 0.2) var(--ion-tab-bar-border-color, var(--ion-border-color, rgba(0, 0, 0, 0.2)))
Tabbar Text Color var(--ion-tabbar-text-color, #8c8c8c) var(--ion-tab-bar-color, var(--ion-text-color-step-550, #8c8c8c))
Tabbar Text Color: Active ion-color(primary, base) var(--ion-tab-bar-color-activated, ion-color(primary, base))
Toolbar Background Color var(--ion-toolbar-background-color, #f8f8f8) var(--ion-toolbar-background, #f8f8f8)
Toolbar Border Color rgba(var(--ion-toolbar-border-color-rgb, 0, 0, 0), 0.2) var(--ion-toolbar-border-color, var(--ion-border-color, rgba(0, 0, 0, 0.2)))
Toolbar Color var(--ion-toolbar-text-color, var(--ion-text-color, #000)) var(--ion-toolbar-color, var(--ion-text-color, #000))
Toolbar Color: Active ion-color(primary, base) var(--ion-toolbar-color-activated, ion-color(primary, base))
Item Background Color var(--ion-item-background-color, var(--ion-background-color, #fff)); var(--ion-item-background, var(--ion-background-color, #fff))
Item Background Color: Active var(--ion-item-background-color-active, #d9d9d9) var(--ion-item-background-activated, #d9d9d9)
Item Border Color var(--ion-item-border-color, #c8c7cc) var(--ion-item-border-color, var(--ion-border-color, #c8c7cc))
Item Text Color var(--ion-item-text-color, var(--ion-text-color, #000)) var(--ion-item-color, var(--ion-text-color, #000))
Placeholder Text Color var(--ion-placeholder-text-color, #999) var(--ion-placeholder-color, var(--ion-text-color-step-600, #999999))

Material Design Defaults

Style Old Value New Value
Backdrop Color var(--ion-backdrop-color, #000)
Border Color var(--ion-border-color, #c1c4cd)
Box Shadow Color var(--ion-box-shadow-color, #000)
Overlay Background Color var(--ion-overlay-background-color, #fafafa)
Tabbar Background Color var(--ion-tabbar-background-color, #f8f8f8) var(--ion-tab-bar-background, #f8f8f8)
Tabbar Background: Focused var(--ion-tabbar-background-color-focused, get-color-shade(#f8f8f8)) var(--ion-tab-bar-background-focused, get-color-shade(#f8f8f8))
Tabbar Border Color rgba(var(--ion-tabbar-border-color-rgb, 0, 0, 0), 0.07) var(--ion-tab-bar-border-color, var(--ion-border-color, rgba(0, 0, 0, 0.07)))
Tabbar Text Color var(--ion-tabbar-text-color, var(--ion-text-color-step-400, #666666)) var(--ion-tab-bar-color, var(--ion-text-color-step-400, #666666))
Tabbar Text Color: Active var(--ion-tabbar-text-color-active, #488aff) var(--ion-tab-bar-color-activated, ion-color(primary, base))
Toolbar Background Color var(--ion-toolbar-background-color, #f8f8f8) var(--ion-toolbar-background, #f8f8f8)
Toolbar Border Color var(--ion-toolbar-border-color, var(--ion-border-color, #c1c4cd))
Toolbar Color var(--ion-toolbar-text-color, #424242) var(--ion-toolbar-color, var(--ion-text-color, #424242))
Toolbar Color: Active var(--ion-toolbar-color-active, #4a4a4a) var(--ion-toolbar-color-activated, #4a4a4a)
Item Background Color var(--ion-item-background-color, var(--ion-background-color, #fff)); var(--ion-item-background, var(--ion-background-color, #fff))
Item Background Color: Active var(--ion-item-background-color-active, #f1f1f1) var(--ion-item-background-activated, #f1f1f1)
Item Border Color rgba(var(--ion-item-border-color-rgb, 0, 0, 0), 0.13) var(--ion-item-border-color, var(--ion-border-color, rgba(0, 0, 0, 0.13)))
Item Text Color var(--ion-item-text-color, var(--ion-text-color, #000)) var(--ion-item-color, var(--ion-text-color, #000))
Placeholder Text Color var(--ion-placeholder-text-color, #999) var(--ion-placeholder-color, var(--ion-text-color-step-600, #999999))
test(themes): modify css-variables test to include more examples
- Adds tabs to test their global variables
- Moves the related theme css files into its test folder
- Comments out the defaults in the defaults file because we need to
test out the fallbacks (leaving commented for now because I may want to
reference them)
- Adds an e2e file to get this test added to screenshot

references #15989
fix(css): update global variables for consistent use
- removes the non mode-specific global Sass variables
- updates the md and ios values so that the default is the css variable
with different fallbacks
- removes non-color related css variables from the global file
- fixes item so it uses the background color that is set by the global
file

references #15989

@brandyscarney brandyscarney requested a review from manucorporat Oct 18, 2018

@brandyscarney

This comment has been minimized.

Copy link
Member Author

commented Oct 18, 2018

@manucorporat This isn't done but I'm requesting review on it early

fix(css): rename/remove global css variables
## Removed Global CSS Variables

The following global CSS variables have been removed for the reasons
listed.

| Variable Name                     | Reason
              |
| ----------------------------------|
------------------------------------------------|
| `--ion-toolbar-color-inactive`    | Unused
              |

## Renamed Global CSS Variables

The following global CSS variables have been renamed for the reasons
listed.

| Old Variable Name                        | New Variable Name
        | Reason
                |
| -----------------------------------------|
-----------------------------------|
------------------------------------------------------------------------
------|
| `--ion-toolbar-text-color`               | `--ion-toolbar-color`
        | Variable is not limited to text color
                |
| `--ion-toolbar-color-active`             |
`--ion-toolbar-color-activated`    | Consistency with our component
variables                                      |
| `--ion-tabbar-text-color`                | `--ion-tabbar-color`
        | Variable is not limited to text color
                |
| `--ion-tabbar-text-color-active`         |
`--ion-tabbar-color-activated`     | Consistency with our component
variables                                      |
| `--ion-tabbar-background-color`          | `--ion-tabbar-background`
        | Applies to the background property
                |
| `--ion-tabbar-background-color-focused`  |
`--ion-tabbar-background-focused`  | Applies to the background property
                                           |
| `--ion-item-background-color`            | `--ion-item-background`
        | Applies to the background property
                |
| `--ion-item-background-color-active`     |
`--ion-item-background-activated`  | Applies to the background property
/ Consistency with our component variables |
| `--ion-item-text-color`                  | `--ion-item-color`
        | Variable is not limited to text color
                |

@brandyscarney brandyscarney requested a review from camwiegert Oct 18, 2018

brandyscarney and others added 2 commits Oct 18, 2018
fix(item): update item fallback to use --ion-border-color
also updates the theme tests to use the new variable names
@ionitron-bot

This comment has been minimized.

Copy link

commented Oct 18, 2018

Screenshot Report 🔎📱

No comparison data found!


💙 - Ionitron

4 similar comments
@ionitron-bot

This comment has been minimized.

Copy link

commented Oct 19, 2018

Screenshot Report 🔎📱

No comparison data found!


💙 - Ionitron

@ionitron-bot

This comment has been minimized.

Copy link

commented Oct 19, 2018

Screenshot Report 🔎📱

No comparison data found!


💙 - Ionitron

@ionitron-bot

This comment has been minimized.

Copy link

commented Oct 19, 2018

Screenshot Report 🔎📱

No comparison data found!


💙 - Ionitron

@ionitron-bot

This comment has been minimized.

Copy link

commented Oct 19, 2018

Screenshot Report 🔎📱

No comparison data found!


💙 - Ionitron

@manucorporat

This comment has been minimized.

Copy link
Member

commented Oct 19, 2018

#Renamed Global CSS Variables
^ i don't understand why we need some of these variables at all

@ionitron-bot

This comment has been minimized.

Copy link

commented Oct 19, 2018

Screenshot Report 🔎📱

No comparison data found!


💙 - Ionitron

1 similar comment
@ionitron-bot

This comment has been minimized.

Copy link

commented Oct 19, 2018

Screenshot Report 🔎📱

No comparison data found!


💙 - Ionitron

@ionitron-bot

This comment has been minimized.

Copy link

commented Oct 19, 2018

Screenshot Report 🔎📱

No comparison data found!


💙 - Ionitron

@ionitron-bot

This comment has been minimized.

Copy link

commented Oct 19, 2018

Screenshot Report 🔎📱

test device images pixels
alert: basic, long message iPhone 1147286
alert: standalone Android 1091639
datetime: standalone, should open basic picker iPhone 876058
segment: standalone Android 24022
datetime: standalone, should open basic picker Android 10270
alert: standalone iPhone 8507
segment: basic Android 4048
label: standalone iPhone 2765
label: standalone Android 2566
item: inputs iPhone 2496

6 mismatched screenshots not shown. See the full comparison.


💙 - Ionitron

--ion-item-border-color: var(--ion-border-color);
--ion-item-text-color: var(--ion-text-color);
--ion-placeholder-text-color: #999;
--ion-tabbar-background: #343d46;

This comment has been minimized.

Copy link
@manucorporat

manucorporat Oct 23, 2018

Member

--ion-tabbar?

brandyscarney added a commit to ionic-team/ionic-docs that referenced this pull request Oct 23, 2018
fix(css): remove --ion-header vars in favor of css
Rather than using `--ion-header-*` variables to update all headings,
just use CSS to style the heading font-size

@brandyscarney brandyscarney merged commit b2021fd into master Oct 23, 2018

7 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: build-core Your tests passed on CircleCI!
Details
ci/circleci: test-core-clean-build Your tests passed on CircleCI!
Details
ci/circleci: test-core-lint Your tests passed on CircleCI!
Details
ci/circleci: test-core-screenshot Your tests passed on CircleCI!
Details
ci/circleci: test-core-spec Your tests passed on CircleCI!
Details
ci/circleci: test-core-treeshake Your tests passed on CircleCI!
Details

@brandyscarney brandyscarney deleted the fix-global-css-variables branch Oct 23, 2018

brandyscarney added a commit to ionic-team/ionic-docs that referenced this pull request Oct 23, 2018
brandyscarney added a commit to ionic-team/ionic-conference-app that referenced this pull request Oct 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.