Ionic-framework: bug: Responsive Display Attributes

Created on 23 Jun 2019  Â·  4Comments  Â·  Source: ionic-team/ionic-framework

Bug Report

Ionic version:
[x] 4.x

Current behavior:
Responsive Display Attributes isn’t working as documented:
.ion-hide-sm-{dir} | Applies the modifier to the element when min-width: 576px (up) or max-width: 576px (down).

Currently, .ion-hide-sm-down hides element on md or less (max-width 767px)

Expected behavior:
is expected to hide the element on a xs breakpoint or smaller (max-width: 576px).

Related code:
https://github.com/ionic-team/ionic/blob/78e477b2a7360537f5aeeca48f14cbf6c55f0a11/core/src/css/display.scss#L24

produces this faulty css:

@media (max-width: 767px) {
  .ion-hide-sm-down {
    display: none !important;
  }
}
core bug

Most helpful comment

@brandyscarney Thanks for the detailed reply.

I actually think the docs should be right. they make more logical sense. @dkrahn thinks the same.
_eg._ if I want an element to be hidden only on a phone, I’d expect to use: ion-hide-sm-down - which is logically: please hide sm and lower (phone and smaller). but with the current code, I’d need to do .ion-hide-down to get that result (which is by itself strange, as it “should” be .ion-hide-xs-down but you have a bug here ).

media-breakpoint-down (ionic.mixins.scss) is only used for this display function. if you pull my PR, you don’t need this mixin anymore, and the compiled display.css is cleaner (try npm run css.sass on my branch)

All 4 comments

Thank you for the issue, but this is a documentation issue not an issue with the code. See the comment above the breakpoint mixin:

// Media of at most the maximum breakpoint width. No query for the largest breakpoint.
// Makes the @content apply to the given breakpoint and narrower.

https://github.com/ionic-team/ionic/blob/master/core/src/themes/ionic.mixins.scss#L123-L134

There is an issue for documentation here: https://github.com/ionic-team/ionic-docs/issues/616

Are you having problems using it or is it the naming/functionality combo that is the problem? https://codepen.io/brandyscarney/pen/bPRLyG

@brandyscarney Thanks for the detailed reply.

I actually think the docs should be right. they make more logical sense. @dkrahn thinks the same.
_eg._ if I want an element to be hidden only on a phone, I’d expect to use: ion-hide-sm-down - which is logically: please hide sm and lower (phone and smaller). but with the current code, I’d need to do .ion-hide-down to get that result (which is by itself strange, as it “should” be .ion-hide-xs-down but you have a bug here ).

media-breakpoint-down (ionic.mixins.scss) is only used for this display function. if you pull my PR, you don’t need this mixin anymore, and the compiled display.css is cleaner (try npm run css.sass on my branch)

@brandyscarney I honestly think you should reconsider this issue and pull request https://github.com/ionic-team/ionic/pull/18601. it's not just my view, see other commentators..
and anyway - the existing rendered css is faulty, so even if you don't take my PR, it still needs fixing.

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

Was this page helpful?
0 / 5 - 0 ratings