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

refactor(components): remove unnecessary css variables #16600

Merged
merged 36 commits into from
Dec 14, 2018

Conversation

brandyscarney
Copy link
Member

@brandyscarney brandyscarney commented Dec 5, 2018

Reviews the --width and --height variables in each component to either remove or add them based on need.

Components

Remove the --width and --height variables from the following:

  • Button
  • FAB Button

Problem Components:

  • Checkbox
    • Checkbox uses the --height variable to determine the border-radius.
    • Solution: add a --size variable that will be set to the width and height, allowing width and height to still be set and border-radius to still use a variable
  • Radio
    • Radio has a --width/--height and an --inner-width/--inner-height. While we could remove the --width and --height, the inner width controls the checked state (iOS checkmark / MD dot). We could just hardcode these, or keep the inner-* variables. I can't find a good way to use the parent width and not expose these.
    • Solution: remove width/height variables for pure css, remove inner variables and calculate based on parent element

Investigate Components:

The following components should be shadow (or scoped) with CSS variables for width/height since the wrapper will need it:

  • Action Sheet (scoped)
  • Alert (scoped)
  • Loading (scoped)
  • Menu (shadow)
  • Modal (scoped)
  • Picker (scoped)
  • Popover (scoped)
  • Toast (shadow)

The above components will now have the following CSS variables for consistency among overlays:

Name
--height
--max-height
--max-width
--min-height
--min-width
--width

If the component does not set the value, it will default to auto.

BREAKING CHANGES

The following CSS properties have been removed:

Component Property Reason
Button --height Use CSS instead
Button --margin-bottom Use CSS instead
Button --margin-end Use CSS instead
Button --margin-start Use CSS instead
Button --margin-top Use CSS instead
Button --width Use CSS instead
Checkbox --height Use CSS or --size
Checkbox --width Use CSS or --size
FAB Button --width Use CSS instead
FAB Button --height Use CSS instead
FAB Button --margin-bottom Use CSS instead
FAB Button --margin-end Use CSS instead
FAB Button --margin-start Use CSS instead
FAB Button `--margin-top Use CSS instead
Menu --width-small Use a media query and --width
Radio --width Use CSS instead
Radio --height Use CSS instead
Radio --inner-height Calculated based on parent
Radio --inner-width Calculated based on parent

Closes #16097

@ionitron-bot ionitron-bot bot added the package: core @ionic/core package label Dec 5, 2018
@brandyscarney
Copy link
Member Author

Radio has been updated, it will now work to update just the width and height of the parent radio, plus updating the border-width on Material Design to scale properly:

.ios .custom-size {
  width: 60px;
  height: 96px;
}

.md .custom-size {
  --border-width: 8px;

  width: 80px;
  height: 80px;
}

screen shot 2018-12-10 at 6 14 43 pm

screen shot 2018-12-10 at 6 32 13 pm

brandyscarney and others added 16 commits December 10, 2018 18:42
- fixes a bug where the spinner color wasn't being set properly
- adds css variables for customizing background, size, spinner color
- fixes a bug where prefix, suffix are taking up too much width
- allows for customizing height, width vars as well as background, color, and border properties
- adds e2e example of calling a picker without datetime
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core @ionic/core package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant