Fix dynamic styling mechanism#1316
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Thanks, @jamesmisson! This looks reasonable to me, but I don't know enough history to say why this is the way it is. Did you try doing a "git blame" on any of the weird offending code to see if that revealed meaningful history? If you'd like me to help with that (or do a demo of how to do it), I'd be happy to fit that in somewhere. |
crhallberg
left a comment
There was a problem hiding this comment.
Looks like a good, clear way to control what components are visible.
|
@jamesmisson, did you mean to add another commit to this PR? Wondering if this should be rolled back to just the first commit so we can merge that when tested/approved, and then we can open a separate PR for the next step. |
fc03b0d to
6a886a6
Compare
|
@demiankatz unintentional! I've rolled that back. I was trying to do a PR that triggers the right panel from the mobile footer button; have now moved that to another branch, I'lll wait to coordinate PRs with @LlGC-jop when the right panel mobile styling is ready. |
|
@jamesmisson, Thanks for the request! I tried testing this, but unfortunately, the deployment on Vercel failed, so no URL was generated for me to check or test the functionality. The preview I highlighted in the attached screenshot also didn’t generate a URL on Vercel for me to preview and test. |
|
@LanieOkorodudu, it looks like my theory from this morning's standup was correct, and the build was broken because the Git repository was in a weird state. @jamesmisson's latest push seems to have sorted it out! |
LanieOkorodudu
left a comment
There was a problem hiding this comment.
@jamesmisson, I’ve tested it, and everything behaves as expected. No issues found on my end

TL;DR: UV wasn't actually picking up styling depending on sm, m, lg, xl screen sizes; panels were appearing/disappearing based on a fall-back default value.
This PR cleans up and enables the mechanism for styling based on small, medium, large, and extra large screen sizes that are defined in variables.less. Previously, this sizing system wasn't actually being activated. Because none of sm, md, lg, or xl.less had media queries checking for those sizes, only the last one to be imported in styles.less was being applied (xl.less). XL defined the visibility with the mobile only/desktop only mixins, but these relied on variables that were only defined if the medium media query was activated in this block of mixins-extended.less:
As this was wrapped in a media query, these variables were only defined if the screen was medium or above! At size small, they weren't defined and so this
defaulted to none and hidden, which was applied to the panels. So, the panels did indeed disappear at the small size, but not via the system that seemed to be intended by the sm, m, lg, and xl.less files. This sounds pretty convoluted, would be happy to demonstrate with dev tools if needed.
This PR cleans this all up a bit and (I hope) makes it a bit more comprehensible as we move on to working on the mobile view. It lets our mobile view (sm) for the panels be styled either in the sm.less, or the panel css file via a media query. This is just a suggestion for a simpler system, but I'm open to suggestions to other ways to organize it.
This PR shouldn't change the current behaviour of UV at different sizes. I've done some manual testing but can give it a more thorough test tomorrow morning. I am wondering if there was a good reason it was done in the hacky way before that will become apparent with further testing. Maybe @crhallberg would be a good person to look at this to see if what I've done makes sense.