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(components): update to include mode classes on missing components #17838

Merged
merged 43 commits into from Apr 16, 2019

Conversation

2 participants
@brandyscarney
Copy link
Member

commented Mar 20, 2019

Short description of what this resolves:

Adds the mode class to the components that don't have it & need it, for example:

<ion-card>

⬇️⬇️⬇️

<!-- in iOS -->
<ion-card class="ios">

<!-- in MD -->
<ion-card class="md">

Removed mode-less component classes from the internal CSS & component:

.slides {
  // some CSS
}

⬇️⬇️⬇️

ion-slides {
  // some CSS
}

Changes proposed in this pull request:

  • Removed the internal component (mode-less) classes from the following (for example, .card-content, .footer) and, if it was being used to target the element in the CSS, replaced it with the element's tag:
    • ion-card-content
    • ion-footer
    • ion-header
    • ion-infinite-scroll-content
    • ion-item-group
    • ion-list
    • ion-picker
    • ion-refresher
    • ion-slides
    • ion-split-pane
  • Added tests to make sure item is added as a class to the following:
    • ion-item
    • ion-item-divider
    • ion-item-group
  • Added tests to make sure button is added as a class to the following:
    • ion-button
    • ion-back-button
    • ion-menu-button
  • Added tests to verify the following have their mode specific classes (e.g. card-content-md) still for internal styling:
    • ion-card-content
    • ion-footer
    • ion-header
    • ion-infinite-scroll-content
    • ion-item-group
    • ion-item-options
    • ion-list
    • ion-picker
    • ion-refresher
    • ion-slides
    • ion-split-pane

Fixes: #17608

Components that do not need mode classes

  • action-sheet-controller
  • alert-controller
  • keyboard-controller
  • loading-controller
  • menu-controller
  • modal-controller
  • nav-pop
  • nav-push
  • nav-set-root
  • nav
  • picker-controller
  • platform
  • popover-controller
  • route-redirect
  • route
  • router-outlet
  • router
  • tab
  • tabs
  • toast-controller
  • virtual-scroll

Components lacking mode classes

- component has classes added for mode (ios, md)
📝 - e2e test checking for classes has been added

Class Test Component Type
📝 action-sheet scoped
📝 alert scoped
📝 anchor shadow
📝 app
📝 avatar shadow
📝 back-button scoped
📝 backdrop shadow
📝 badge shadow
📝 button shadow
📝 buttons scoped
📝 card-content
📝 card-header shadow
📝 card-subtitle shadow
📝 card-title shadow
📝 card scoped
📝 checkbox shadow
📝 chip shadow
📝 col
📝 content shadow
📝 datetime shadow
📝 fab-button shadow
📝 fab-list shadow
📝 fab
📝 footer
📝 grid shadow
📝 header
📝 icon
📝 img shadow
📝 infinite-scroll
📝 infinite-scroll-content
📝 input scoped
📝 item-divider shadow
📝 item-option shadow
📝 item-options
📝 item-sliding
📝 item shadow
📝 item-group
📝 label scoped
📝 list
📝 list-header shadow
📝 loading scoped
📝 menu-button shadow
📝 menu-toggle shadow
📝 menu shadow
📝 modal scoped
📝 note shadow
📝 picker scoped
📝 picker-column
📝 popover scoped
📝 progress-bar shadow
📝 radio-group
📝 radio shadow
📝 range shadow
📝 refresher
📝 refresher-content
📝 reorder-group
📝 reorder shadow
📝 ripple-effect shadow
📝 row shadow
📝 searchbar scoped
📝 segment-button shadow
📝 segment scoped
📝 select-option shadow
📝 select-popover scoped
📝 select shadow
📝 skeleton-text shadow
📝 slide
📝 slides shadow
📝 spinner shadow
📝 split-pane
📝 tab-bar shadow
📝 tab-button shadow
📝 text shadow
📝 textarea scoped
📝 thumbnail shadow
📝 title shadow
📝 toast shadow
📝 toggle shadow
📝 toolbar shadow

brandyscarney added some commits Mar 20, 2019

@liamdebeasi
Copy link
Member

left a comment

Looking really good!

Show resolved Hide resolved core/src/components/card/test/basic/e2e.ts Outdated
Show resolved Hide resolved core/src/components/tabs/test/standalone/e2e.ts
Show resolved Hide resolved core/src/components/card/test/basic/e2e.ts Outdated

@brandyscarney brandyscarney marked this pull request as ready for review Mar 21, 2019

brandyscarney added some commits Mar 22, 2019

brandyscarney added some commits Mar 25, 2019

@liamdebeasi
Copy link
Member

left a comment

looks great! 🎉

Show resolved Hide resolved core/src/utils/test/utils.ts Outdated
Show resolved Hide resolved core/src/utils/test/utils.ts Outdated
@brandyscarney

This comment has been minimized.

Copy link
Member Author

commented Mar 26, 2019

Screen Shot 2019-03-26 at 2 01 57 PM

this broke the toolbar/scenario test

@brandyscarney

This comment has been minimized.

Copy link
Member Author

commented Mar 26, 2019

The bug in my last comment is directly related to #17876

brandyscarney added some commits Apr 10, 2019

@brandyscarney

This comment has been minimized.

Copy link
Member Author

commented Apr 10, 2019

@liamdebeasi Could you take another look at this? I think the classes themselves are all good, I added it to ionicons and updated the dependency. I reverted the change with title because I think trying to remove the mode classes may be a breaking change and I want to investigate that more.

@brandyscarney brandyscarney requested a review from liamdebeasi Apr 10, 2019

@brandyscarney brandyscarney added this to In progress 🤺 in Ionic Core via automation Apr 10, 2019

@brandyscarney brandyscarney moved this from In progress 🤺 to Needs review 🤔 in Ionic Core Apr 10, 2019

brandyscarney added some commits Apr 15, 2019

@brandyscarney brandyscarney merged commit e5c8c10 into master Apr 16, 2019

2 checks passed

build Workflow: build
Details
screenshot Screenshot
Details

Ionic Core automation moved this from Needs review 🤔 to Done 🎉 Apr 16, 2019

@brandyscarney brandyscarney deleted the test-updates branch Apr 16, 2019

Kiku-git added a commit to Kiku-git/ionic that referenced this pull request May 16, 2019

fix(components): include mode classes on components for use in shadow (
…ionic-team#17838)

- removes mode-less component classes from the internal CSS, use element instead
- adds mode specific classes `md` or `ios` for styling inside of shadow components
- adds e2e test that verifies mode classes exist on all ionic components, plus checks for specific classes that the components need for internal styling

fixes ionic-team#17608
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.