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.

Sign up for GitHub

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

Jump to bottom

feat(components): improve hover states for buttons #19440

Merged
merged 120 commits into from Jan 23, 2020
Merged

feat(components): improve hover states for buttons #19440

merged 120 commits into from Jan 23, 2020

Conversation

brandyscarney
Copy link
Member

@brandyscarney brandyscarney commented Sep 24, 2019 •
edited

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build (npm run build) was run locally and any changes were pushed
  • Lint (npm run lint) has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Currently users have to know the exact opacity that the MD spec uses in order to change the hover color. This allows them to change the background without having to know the opacity. There is also a bug with dark mode because the hover state would need to be set to the toolbar color with the opacity for each fill.

References #18279 Fixes #20213 Fixes #19965

What is the new behavior?

  • greatly reduces the requirement by users to set the background hover for dark modes, also eliminates the need to know what the hover opacity is for each button fill
  • updates the MD dark theme per their spec

Component Updates

Any component in the below table needs to have hover corrected.

Table Key:

  • Existing Hover (master): Did the component have the variables for --background-hover on master
  • Updated Hover?: Have the hover variables been updated for this branch

⬜ - Incomplete
✅ - Done
🌀 - Doesn't exist

Name Existing Hover (master) Updated Hover? Updated Activated? Updated Focused?
Action Sheet (Buttons) ⬜ ✅ ✅ ✅
Back Button ✅ ✅ 🌀 ✅
Button ✅ ✅ ✅ ✅
FAB Button ✅ ✅ ✅ ✅
Item ✅ ✅ ✅ ✅
Menu Button ✅ ✅ 🌀 ✅
Segment Button ✅ ✅ 🌀 ✅
Tab Button ⬜ ⬜ ⬜ ✅

Remaining components here: #18279

Does this introduce a breaking change?

  • Yes
  • No

Will go into Ionic 5.

BREAKING CHANGES

Activated Class

The activated class that is automatically added to buttons on press has been renamed to ion-activated. This will be more consistent with our ion-focused class we add and also will reduce conflicts with user's CSS.

CSS Variables

The --background-hover CSS variable on buttons will now have an opacity automatically set. Anyone setting it like the following:

--background-hover: rgba(44, 44, 44, 0.08);

Will likely not see a hover state anymore, it should be updated to the following:

--background-hover: rgba(44, 44, 44);

If the opacity desired is something other than what is set by default, use:

--background-hover: rgba(44, 44, 44);
--background-hover-opacity: 1;

Other information

liamdebeasi and others added 3 commits September 20, 2019 16:42
@liamdebeasi
refactor(toast): remove showCloseButton and closeButtonText (#18957)
bc324c4 
* Remove close button

* update tests

* update tests

* add build
@manucorporat
refactor(): deprecate web component controllers (#19109)
bc99f4e
feat(components): update hover states for buttons
cbd46e6 
- applies to ion-back-button, ion-menu-button, ion-button
- still need to do others including fab
- CSS cleanup
- this is only implemented in MD
- greatly reduces the requirement by users to set the background hover for dark modes, also eliminates the need to know what the hover opacity is for each
- updates the MD dark theme per their spec
@ionitron-bot ionitron-bot bot added package: core @ionic/core package package: vue @ionic/vue package labels Sep 24, 2019
@brandyscarney brandyscarney changed the base branch from master to next September 24, 2019 18:33
Brandy Carney added 13 commits September 24, 2019 18:00
fix(components): updates to disabled
6230408
fix(tap-click): rename activated class to ion-activated
85f9bf0 
BREAKING CHANGE css class `activated` -> `ion-activated` to stay consistent with `ion-focused` and reduce conflicts with user's CSS

Also fixes an issue with border radius, the hover state for buttons w/ color
docs(component-guide): add info on activated, focused, ripple
7e74566
style(lint): fix lint issues
eb89c65
fix(fab): update to button hover
d42c152
fix(action-sheet): add hover state
b03df32
feat(item-option): add hover support
cdcd0c5
fix(item-option): fix missing CSS for hover
586c353
docs(guide): update component guide to include converting scoped
d169c45
style(): ::after
66a340d
feat(chip): add background hover vars
5eb0ad1
test(css): update toast to include close button
70152bc
feat(picker): add hover states for picker buttons
b2efcb8
liamdebeasi and others added 10 commits October 7, 2019 13:16
@liamdebeasi
refactor(toast): remove showCloseButton and closeButtonText (#18957)
cc66486 
* Remove close button

* update tests

* update tests

* add build
@manucorporat
refactor(): deprecate web component controllers (#19109)
ae8a69e
@brandyscarney
refactor(nav): remove nav-push, nav-pop nav-root (#19240)
64f8d3c 
BREAKING CHANGES

Removes ion-nav-pop, ion-nav-push and ion-nav-set-root in favor of using ion-nav-link with router-direction
@brandyscarney
refactor(scss): remove scss (#19292)
679fe4b 
BREAKING CHANGES

Removes `scss` files from the distributed files. Please use CSS variables for theming instead.
@brandyscarney
refactor(themes): update to new ionic colors (#19279)
ef95345 
BREAKING CHANGES

The Ionic default colors have been updated to the following:

primary:         #3880ff
secondary:       #3dc2ff
tertiary:        #5260ff
success:         #2dd36f
warning:         #ffc409
danger:          #eb445a
light:           #f4f5f8
medium:          #92949c
dark:            #222428

`primary`, `light` and `dark` have not changed. The contrast color for `warning` has been updated to `#000`.
@brandyscarney
refactor(css): remove CSS util attributes (#18956)
16f179a 
BREAKING CHANGES

Removes all CSS utility attributes. Please use CSS classes instead. See the documentation for the correct class names: https://ionicframework.com/docs/layout/css-utilities
@brandyscarney
refactor(skeleton-text): remove width property (#18936)
3e470be 
BREAKING CHANGES

Skeleton text's `width` property has been removed. Please use CSS instead to set the width.
@brandyscarney
refactor(anchor): remove anchor in favor of router-link (#18935)
9a679d8 
BREAKING CHANGES

The deprecated `ion-anchor` component has been removed in favor using `ion-router-link`. It should still only be used with vanilla and Stencil JavaScript projects. For Angular projects, use an `<a>` and `routerLink` with the Angular router.
@brandyscarney
refactor(searchbar): remove boolean values from showCancelButton (#18953
6e602a4 
)

BREAKING CHANGES

The `show-cancel-button` property of the searchbar no longer accepts boolean values. Accepted values are strings: `"focus"`, `"always"`, `"never"`. The following should change:

```
<ion-searchbar show-cancel-button>
<ion-searchbar show-cancel-button="true">
<ion-searchbar show-cancel-button="false">
```

becomes

```
<ion-searchbar show-cancel-button="focus">
<ion-searchbar show-cancel-button="focus">
<ion-searchbar show-cancel-button="never">
```
@liamdebeasi
refactor(): remove no-border references, add ion-no-border class (#18954
4716a27 
)

* remove no border ref

* change to dispaly

* update usage
@brandyscarney
Copy link
Member Author

@liamdebeasi

I am going through all of these now, but wanted to provide some initial feedback on action sheet so you're not waiting on me to finish my review:

Action Sheet

  1. iOS only: --button-background does not seem to work. background-color: transparent appears to be overriding it.

Fixed by: 1d4fe52

liamdebeasi
liamdebeasi reviewed Jan 23, 2020
View reviewed changes
BREAKING.md Outdated Show resolved Hide resolved
BREAKING.md Show resolved Hide resolved
BREAKING.md Outdated Show resolved Hide resolved
BREAKING.md Outdated Show resolved Hide resolved
BREAKING.md Outdated Show resolved Hide resolved
BREAKING.md Show resolved Hide resolved
BREAKING.md Outdated Show resolved Hide resolved
BREAKING.md Outdated Show resolved Hide resolved
liamdebeasi
liamdebeasi approved these changes Jan 23, 2020
View reviewed changes
Copy link
Member

@liamdebeasi liamdebeasi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A++

@brandyscarney
Copy link
Member Author

Ready 2 Merge

@brandyscarney brandyscarney merged commit 9415929 into master Jan 23, 2020
@brandyscarney brandyscarney deleted the fix-hover-states branch January 23, 2020 21:57
@brandyscarney brandyscarney mentioned this pull request Jan 23, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liamdebeasi liamdebeasi liamdebeasi approved these changes

Assignees

@brandyscarney brandyscarney

Labels
package: core @ionic/core package package: vue @ionic/vue package
Projects
None yet
Milestone
No milestone
5 participants
@brandyscarney @liamdebeasi @manucorporat @adamdbradley @oori