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

bug: remove global hidden attribute styles #17583

Closed
brandyscarney opened this issue Feb 22, 2019 · 4 comments
Closed

bug: remove global hidden attribute styles #17583

brandyscarney opened this issue Feb 22, 2019 · 4 comments
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@brandyscarney
Copy link
Member

Bug Report

Ionic version:

[x] 4.x

Current behavior:
We have the following CSS in the global core stylesheet:

[hidden] { 
  display: none !important;
}

This causes conflicts with the native hidden attribute, see the MDN docs here: https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/hidden

screen shot 2019-02-22 at 3 13 36 pm

Expected behavior:
This style should not be there or it should not use !important. It may be seen as a breaking change to remove it but I'd argue it is a fix to a problem because it interferes with the native functionality of the attribute. Up for discussion.

Steps to reproduce:

  1. Open this Codepen: https://codepen.io/brandyscarney/pen/XOLLLj
    • Verify the div is not displaying even though it has display block (adding !important does not override either)
  2. Open this Codepen: https://codepen.io/brandyscarney/pen/YBooma
    • See that the div is displaying properly when Ionic is not included

Other information:
I stumbled on this while reviewing the following PR: #17359

@ionitron-bot ionitron-bot bot added the triage label Feb 22, 2019
@brandyscarney brandyscarney added package: core @ionic/core package type: bug a confirmed bug report labels Feb 22, 2019
@ElNinjaGaiden
Copy link

I would say, more than only remove the important! config, this has to be removed at all, or treated in a different way.

On my code I do have an IonButton (@ionic/react) where I'm binding the hidden button property to a value on handled by the state of my main/parent component (the component that holds the IonButton) and even after my flag on the state is set to false and I get this code rendered:

Screen Shot 2019-06-09 at 3 52 12 PM

Even having the value of the property set to false, the button is inheriting the whole set of styles.

This, or, at least for that specific use case, the render logic should remove the hidden attribute at all instead of keep sending it as hidden="false" because the CSS is not using the attribute value in consideration due this

@brandyscarney
Copy link
Member Author

@ElNinjaGaiden Yes, I think that it should be removed entirely since it interferes with the functionality of the native attribute. My only concern is that it may be a breaking change to some apps if we remove this, so I would need to do some more thorough testing on it.

@brandyscarney brandyscarney added this to the Next milestone Aug 27, 2019
@liamdebeasi liamdebeasi modified the milestones: Next, 6.0.0 Jun 16, 2020
@liamdebeasi liamdebeasi removed this from the 6.0.0 milestone Jul 16, 2021
@liamdebeasi liamdebeasi added type: bug a confirmed bug report and removed type: bug a confirmed bug report labels May 23, 2022
liamdebeasi added a commit that referenced this issue Aug 26, 2022
resolves #17583

BREAKING CHANGE: 

The `[hidden]` attribute has been removed from Ionic's global stylesheet. The `[hidden]` attribute can continue to be used, but developers will get the [native `hidden` implementation](https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/hidden) instead. The main difference is that the native implementation is easier to override using `display` than Ionic's implementation.

Developers can add the following CSS to their global stylesheet if they need the old behavior:

```css
[hidden] {
  display: none !important;
}
```
@liamdebeasi
Copy link
Contributor

This was fixed in #25829. Please note that since this is a breaking change, the fix will be available in Ionic 7, the next major release.

@ionitron-bot
Copy link

ionitron-bot bot commented Sep 25, 2022

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.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Sep 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Development

No branches or pull requests

3 participants