Add a button to expand and contract the radar #1393
Conversation
| @@ -1,12 +1,57 @@ | |||
| @use "uswds-core" as *; | |||
|
|
|||
| .wx-radar-container { | |||
| aspect-ratio: 3/4; | |||
| aspect-ratio: 3/2; | |||
There was a problem hiding this comment.
The design calls for a starting aspect ratio of 6/4 on mobile and 3/2 on tablet+, but AFAIK those are the same number. 😆
| } | ||
|
|
||
| button.wx-radar-expand { | ||
| @include u-bg("transparent"); |
There was a problem hiding this comment.
I think this is actually not strictly necessary anymore. I added this when I was trying to get the button to be the same height as its contents, before I learned about the baseline vertical alignment magic margin. But I suppose it doesn't hurt anything, either?
| </wx-radar> | ||
|
|
||
| <div class="position-absolute top-0 right-0 height-full"> | ||
| <div class="position-sticky top-0" style="top: 55px;"> |
There was a problem hiding this comment.
top needs to account for the height of the stickied tab list. It's about 55 pixels, so use that as a default, though the very actually real value is found and set in Javascript.
There was a problem hiding this comment.
LGTM 👍
I have one inline suggestion about the whole position-sticky thing, though I might be missing something in that regard
Code Review Checklist
This is an automated comment on every pull request requiring a review. A checked item is either an assertion that I have tested this item or an indication that I have verified it does not apply to this pull request.
The Basics
- Checks are passing
- I read the code
- I ran the code
- (if applicable) Post deploy steps are run
- (if applicable) I validated the change on the deployed version in
Documentation
- changes to “how we do things” are documented in READMEs
- all new functions and methods are commented using plain language
- any new modules added documented in modules.md
Security
- security false positives are documented
- data from external sources is cleaned and clearly marked
Reliability
- error handling exists for unusual or missing values
- interactions with external systems are wrapped in try/except
- functionality is tested with unit or integration tests
- dependency updates in composer.json also got changed in composer-lock.json
Infrastructure
- all changes are auditable and documented via a script
- it is clear who can and should run the script
- (if applicable) diagrams have been updated or added in PlantUML
Accessibility
- New pages have been added to cypress-axe file so that they will be tested with our automated accessibility testing
- Meets WCAG 2.0 AA or 2.1 AA for Section 508 compliance
- Site is keyboard accessible. All interactions can be accessed with a keyboard
- Site is free of keyboard traps. The keyboard focus is never trapped in a loop
- All form inputs have explicit labels
- Form instructions are associated with inputs
- All relevant images use an img tag
- All images have appropriate alt attributes
- Multimedia is tagged. All multimedia has appropriate captioning and audio description
- Text has sufficient color contrast. All text has a contrast ratio of 4.5:1 with the background
- Site never loses focus. Focus is always visible when moving through the page with the keyboard
- Tab order is logical
- Tables are coded properly. Tables have proper headers and column attributes
- Headings are nested properly. Heading elements are nested in a logical way
- Language is set. The language for the page is set
- CSS is not required to use the page. The page makes sense with or without CSS
- Links are unique and contextual. All links can be understood taken alone, e.g., ‘Read more - about 508’
- Page titles are descriptive
Device Matrix
- firefox/gecko (renders correctly and user interactions work)
- chrome/chromium/edge (renders correctly and user interactions work)
- safari/webkit (renders correctly and user interactions work)
- web page is readable and usable
- at 480px (mobile)
- at 640px (tablet)
- at 1024px (desktop)
| for (const descriptor of descriptors) { | ||
| if (descriptor.classList.contains("display-none")) { | ||
| descriptor.classList.remove("display-none"); | ||
| } else { | ||
| descriptor.classList.add("display-none"); | ||
| } | ||
| } |
There was a problem hiding this comment.
there is also classList.toggle() which might do the same thing
| <div class="position-absolute top-0 right-0 height-full"> | ||
| <div class="position-sticky top-0" style="top: 55px;"> |
There was a problem hiding this comment.
Just playing around with this, I'm wondering if there needs to be the outer position-absolute div at all. I'm thinking you could just have the inner one, have it be positioned absolutely, and give it top and right values of like 5px or something. That might accomplish the same thing without all the worry of sticky and you won't need the inline style attribute at all -- you could get rid of that code in the js file
| <div class="position-absolute top-0 right-0 height-full"> | |
| <div class="position-sticky top-0" style="top: 55px;"> | |
| <div class="position-absolute top-2px right-2px"> |
There was a problem hiding this comment.
(top-0 and right-0 will also work, due to the positioning of the SVG within the containing div there is already some margin)
|
Looking good, just a few design notes:
|
|
@partly-igor I think 1, 2, and 3 are fixed. I similarly saw 4 for a while last night and it was related to which element was receiving the click event. I can't reproduce it anymore, but if you see it again, definitely let me know so we can dig into it. |
What does this PR do? 🛠️
I had fun playing with CSS, except for the surprise discovery that inline elements (and inline-block elements as well) have a default vertical alignment of
baselinewhich causes them to have an un-stylable small bottom margin. Still, good to know, I reckon. Otherwise, I don't think there's anything remarkable here, code-wise.Curious if we might later want to add a transition to the aspect ratio change, but I'm not sure how nicely that would play with the window resize event. Maybe something to experiment with, if we're interested.
Screenshots (if appropriate): 📸
Screen.Recording.2024-06-26.at.2.09.41.PM.mov