fix(css): Responsive Display Attributes

[oori]

See: #18600

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

What is the current behavior?

Issue Number: 18600

What is the new behavior?

Responsive Display Attributes now works as documented. the compiled display.css is now:

.ion-hide {
  display: none !important;
}

@media (max-width: 576px) {
  .ion-hide-sm-down {
    display: none !important;
  }
}
@media (min-width: 576px) {
  .ion-hide-sm-up {
    display: none !important;
  }
}
@media (max-width: 768px) {
  .ion-hide-md-down {
    display: none !important;
  }
}
@media (min-width: 768px) {
  .ion-hide-md-up {
    display: none !important;
  }
}
@media (max-width: 992px) {
  .ion-hide-lg-down {
    display: none !important;
  }
}
@media (min-width: 992px) {
  .ion-hide-lg-up {
    display: none !important;
  }
}
@media (max-width: 1200px) {
  .ion-hide-xl-down {
    display: none !important;
  }
}
@media (min-width: 1200px) {
  .ion-hide-xl-up {
    display: none !important;
  }
}

Does this introduce a breaking change?

• Yes
• No

[oori]

Also related with ionic-team/ionic-docs#616

[abennouna]

Also related with ionic-team/ionic-docs#616

Thanks for the issue! I moved this to our ionic-docs repository as the code looks fine but the documentation needs to be updated. 🙂

This fix would be a breaking change, considering that the code is correct, but the docs aren't.

[oori]

@abennouna Thanks, but I’m not sure the code is correct. Not simply conceptually, as I and @dkrahn claim (see: #18600 (comment)), but the currently rendered css is faulty:

.ion-hide-up {
    display: none !important
}

@media (max-width: 575px) {
    .ion-hide-down {
        display:none !important
    }
}

@media (min-width: 576px) {
    .ion-hide-sm-up {
        display:none !important
    }
}

Currently, hiding content on phones mean using: .ion-hide-down(no, -sm-down, and .ion-hide-up is hide all…). @brandyscarney, this seems like a hack, and future developer looking at this code, would never know we intended .ion-hide-down css style specifically for phones.

Regarding “breaking change”, I doubt many would get affected. display.css is an optional dependency. The docs really make more sense, then using .ion-hide-down for phones.

[codercatdev]

Agreed, this makes more sense! I just stopped using this and changed to my own, which takes into account the global grid attributes

--ion-grid-width | Width of the fixed Grid
--ion-grid-width-lg | Width of the fixed Grid on lg screens
--ion-grid-width-md | Width of the fixed Grid on md screens
--ion-grid-width-sm | Width of the fixed Grid on sm screens
--ion-grid-width-xl | Width of the fixed Grid on xl screens
--ion-grid-width-xs | Width of the fixed Grid on xs screens

[brandyscarney]

Thank you for the PR! So I made some changes to the media-breakpoint-down function so that it will automatically look for the min of the current breakpoint and use that. I also changed the base of the PR so that it would look at the next branch as this will be going into a major release since it could cause breaking changes in apps that are relying on it to work this way. Here's the new generated CSS, I left in the ion-hide-up and ion-hide-down as that is how all of the other CSS utility files work, but we can optionally remove them:

.ion-hide {
  display: none !important;
}

.ion-hide-up {
  display: none !important;
}

.ion-hide-down {
  display: none !important;
}

@media (min-width: 576px) {
  .ion-hide-sm-up {
    display: none !important;
  }
}
@media (max-width: 576px) {
  .ion-hide-sm-down {
    display: none !important;
  }
}
@media (min-width: 768px) {
  .ion-hide-md-up {
    display: none !important;
  }
}
@media (max-width: 768px) {
  .ion-hide-md-down {
    display: none !important;
  }
}
@media (min-width: 992px) {
  .ion-hide-lg-up {
    display: none !important;
  }
}
@media (max-width: 992px) {
  .ion-hide-lg-down {
    display: none !important;
  }
}
@media (min-width: 1200px) {
  .ion-hide-xl-up {
    display: none !important;
  }
}
@media (max-width: 1200px) {
  .ion-hide-xl-down {
    display: none !important;
  }
}

Let me know your thoughts, thanks!!

[oori]

@brandyscarney Thanks. Yes, the rendered css is correct.

remove .ion-hide-up and .ion-hide-down

Yea, remove them. it doesn't make any sense to use those classes. (one should use ion-hide or specify a responsive size, like: .ion-hide-md-down

[brandyscarney]

So I'm going to keep them for now, removing them would require a change to the media query used to generate this and all of the other utils, and I would rather keep it for consistency. The other CSS util files do something similar, it just doesn't look "as off" because we add ion-hide as an extra class for this util file.

For example, the text transformation classes generate:

.ion-text-uppercase {
  text-transform: uppercase !important;
}

.ion-text-lowercase {
  text-transform: lowercase !important;
}

.ion-text-capitalize {
  text-transform: capitalize !important;
}

instead of using the xs- prefix. That's the same thing that is happening here, the ion-hide-up and ion-hide-down are generated instead of the xs- prefix and >xl.

[dylanvdmerwe]

LOVE this change thank you.

[nickwinger]

for me this is still not working (having ionic version 4.11.10), when applying the ion-hide-sm-down css class i expect the element to be hidden under 576px like stated here: https://ionicframework.com/docs/layout/css-utilities#ionic-breakpoints

but it uses the md breakpoint pixels which is wrong...

this is the wrong css generated:
@media (max-width: 767px)
.ion-hide-sm-down {
display: none !important;
}

[liamdebeasi]

@nickwinger This change was not added to Ionic 4.11.10. It will be available in the next major release of Ionic Framework.