Skip to content

ETT-1542: 'item viewability' sometimes doesn't fit viewport - #155

Merged
carylwyatt merged 4 commits into
mainfrom
ETT-1542
Sep 10, 2026
Merged

ETT-1542: 'item viewability' sometimes doesn't fit viewport#155
carylwyatt merged 4 commits into
mainfrom
ETT-1542

Conversation

@carylwyatt

@carylwyatt carylwyatt commented Sep 4, 2026

Copy link
Copy Markdown
Member

When the enumchron on a volume in the catalog record 'item viewability' table is very long, the table refuses to collapse enough to fit narrow viewports (320px), causing a horizontal scrollbar to appear at narrow viewport widths and failing WCAG 1.4.10 Reflow.

My first attempt at fixing this involved a container query that was overly complicated. It took me way too long to realize that removing the flex container on the links here would fix most of the issue. I was having trouble testing enough edge cases on dev-3 without updating the file every time, so I added a mobile story with some of the edge cases we encountered, and it made it so much easier to see and test. You can see the new mobile story here: https://656a2bfa011def621f569319-raxctfvmug.chromatic.com/?path=/story/designs-table--mobile-default

I also got tripped up by the prettier formatting github action when I pushed this up. Somehow my husky pre-commit didn't catch a file that wasn't formatted. I added a new script to package.json to do the formatting that is found during npm run format:check.

Similar to the catalog PR, Gayathri already approved the designs, UI, and a11y for this fix, but I'd appreciate a glance at the css. Thanks!

@carylwyatt
carylwyatt requested a review from eumalin September 9, 2026 16:23
Comment thread src/scss/apps.scss
}
}

table.viewability-table a {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This overrides the flex display on .item-link above, so gap-2 stops working. The lock icon and text will lose their spacing. Can we check that?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, item-link and gap-2 were leftovers from a previous version and didn't end up making it into the markup in catalog. I'll redo this to match hathitrust/catalog#144

@carylwyatt carylwyatt Sep 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol, those classes were only on one catalog item in the story 💀 they're gone now!

export const MobileDefault = {
globals: {
viewport: {
value: 'bsXs',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bsXs is 400px wide, but the PR targets 320px for WCAG 1.4.10 reflow. This story won't catch an overflow that only happens at 320px. Can we test at 320px?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes a lot of sense! Good catch.

@carylwyatt
carylwyatt requested a review from eumalin September 10, 2026 16:45
Comment thread .storybook/preview.js
styles: {
width: '400px',
width: '336px',
height: '800px',

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Storybook injects a total of 16px width padding around our stories, so I adjusted this up to 336px (320 + 16) to account for that. We could instead add a decorator that would remove storybook's padding, but this felt easier for now.

@eumalin eumalin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thank you!

@carylwyatt
carylwyatt merged commit dff79e3 into main Sep 10, 2026
6 checks passed
@carylwyatt
carylwyatt deleted the ETT-1542 branch September 10, 2026 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants