-
Notifications
You must be signed in to change notification settings - Fork 1.1k
USWDS - [date-picker]: Fix formatting on narrow parent containers #6388
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
base: develop
Are you sure you want to change the base?
Conversation
| el.querySelector(`.${CALENDAR_MONTH_LABEL_CLASS_NARROW}`); | ||
|
|
||
| // Add narrow class styling when parent width is less than 300 | ||
| if (parent.offsetWidth < 300) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I picked 300 based on the bug example that was provided and felt this was the smallest parent size the styles should be switched at before it started looking bad but I'm open to other opinions here. The same styles were previously being applied at "mobile" screen widths.
| // --- Global override to track "resize" event listeners --- | ||
|
|
||
| let resizeListeners = []; | ||
|
|
||
| const originalAddEventListener = window.addEventListener; | ||
| const originalRemoveEventListener = window.removeEventListener; | ||
|
|
||
| // Override addEventListener before each test to capture all "resize" listeners | ||
| beforeEach(() => { | ||
| resizeListeners = []; | ||
| window.addEventListener = function (type, listener, options) { | ||
| if (type === "resize") { | ||
| resizeListeners.push(listener); | ||
| } | ||
| return originalAddEventListener.call(window, type, listener, options); | ||
| }; | ||
| }); | ||
|
|
||
| // Remove captured listeners | ||
| afterEach(() => { | ||
| resizeListeners.forEach((listener) => { | ||
| originalRemoveEventListener.call(window, "resize", listener); | ||
| }); | ||
| resizeListeners = []; | ||
| window.addEventListener = originalAddEventListener; | ||
| window.removeEventListener = originalRemoveEventListener; | ||
| }); | ||
|
|
||
| // --- End Global override --- | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This global clean up method was added so the resize event listener doesn't stick around and get called again on subsequent unit tests that use resize events, which was previously causing failures.
I only added it to this file because it's the last one to run that causes the issue of all the date-picker unit tests. If this file were to be removed for some reason this logic would need to be moved to one of the other files. I was unsure if I should include it in every date-picker test file because there were quite a few and it's technically only needed at the end. LMK if anyone feels differently!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mlloydbixal
Thanks for working on this. I like the idea to check for the parent container width rather than relying on viewport width. However, I am curious if there is a non-JS approach to this issue. Could CSS container queries possibly work here?
Update: More discussion on this can be found in this Slack thread (🔒).
Summary
Enhanced date picker responsiveness Replaced media query-based styling with JavaScript-driven approach to adapt month/year labels to parent container width to prevent overlap issues.
Breaking change
This is not a breaking change.
Related issue
Closes #6069
Related pull requests
None
Preview link
Preview link
Problem statement
The date picker component was experiencing a layout issue where the year button would overlap with the days below when opened, causing a poor user experience.
Solution
By using JavaScript to check the parent container width size, the appropriate styles are applied to resolve the layout issue, rather than relying on CSS media queries.
Major changes
checkWidth()to check parent container width sizeTesting and review
Sandbox for reproducing exact 2 column layout bug