sul-dlss / wallscreens

📺 curated experiences for touch-screen installations on the stanford campus
Other
1 stars 0 forks source link

Use html5 hidden attribute instead of utility class #168

Closed thatbudakguy closed 3 years ago

thatbudakguy commented 3 years ago

this PR isn't strictly necessary, but is related to #87 (and could maybe close it). since we only have one remaining utility class, for showing and hiding things, i decided to take the bootstrap approach and just use the html5 "hidden" attribute instead. the styles we included from reboot mean that this works without needing any more CSS, so we can scrap the utility class.

feel free to treat this as a place for discussion rather than just merging, and I'm open to not doing it – just figured why not since it was more or less a simple find-and-replace, and then we could potentially lay #87 to rest.

would appreciate someone thoroughly poking the netlify preview too; i tested this locally and it seemed not to change anything but it's tough to be sure.

netlify[bot] commented 3 years ago

✔️ Deploy Preview for sul-wallscreens ready!

🔨 Explore the source changes: cfcf86d35c90d0dfa3e238c678e380cab4fafb04

🔍 Inspect the deploy log: https://app.netlify.com/sites/sul-wallscreens/deploys/6184641821301d000768fbf0

😎 Browse the preview: https://deploy-preview-168--sul-wallscreens.netlify.app

cbeer commented 3 years ago

Bootstrap's reboot says:

HTML5 adds a new global attribute named [hidden], which is styled as display: none by default. Borrowing an idea from PureCSS, we improve upon this default by making [hidden] { display: none !important; } to help prevent its display from getting accidentally overridden.

and adds:

// Hidden attribute
//
// Always hide an element with the `hidden` HTML attribute.

[hidden] {
  display: none !important;
}

I'm 👍 to trying hidden, but probably worth grabbing that hidden styling?

thatbudakguy commented 3 years ago

we already have it: https://github.com/sul-dlss/wallscreens/blob/06937e22f0697ecc3735a9ae3593e67759b4af30/_scss/wallscreens/_global.scss#L14-L17

i think this PR would make things look super messed up if we didn't! it's one of the things from reboot that I kept.