Add styles for header - Githubissues

RockefellerArchiveCenter / styles

Style Library for the Rockefeller Archive Center

https://styles.rockarch.org

MIT License

0 stars 1 forks source link

Add styles for header #93

Closed HaSistrunk closed 2 years ago

HaSistrunk commented 3 years ago

Fixes #60

I've divided these styles up into the _header.scss layout, and _dropdown.scss, and _nav.scss component files. The Dropdown component can be used outside of the header context, so I'm going to create an issue to add that component separately to storybook.

In Storybook in the "with Dropdown Items" canvas, you can see by toggling the "showDropdownItems" control that the dropdown is pushing the header buttons around. I can't figure out how to prevent that, but have added it to the known issues section of the documentation (it may be related to Storybook).

helrond commented 3 years ago

For the social icons, I think I would suggest seeing if you can do something to the containing div to make the icons smaller. It may be that some changes need to be made to the social icons component to inherit a font size/set a default if it doesn't exist or something like that. In terms of how they get handled for mobile - my read of that is that the social icons are part of the dropdown items (i.e. they're in the HTML there) and that they get hidden when that collapse occurs. So I think that just means hiding the containing div?

I also tried to figure out what's going on with the dropdown pushing things around, and I don't have an answer. My suggestion would be to add this as a known issue on the component with the caveat that this might be a Storybook thing. I'm noticing that it totally re-renders the HTML when that switch is toggled so I am suspicious it might have something to do with Storybook specifically, rather than the styles.

helrond commented 2 years ago

OK, I did two things:

removed the aria-hidden from the material inside of the mobile nav items. Storybook was complaining that button didn't have readable text.
Flipped the hideMobileNavItems to showMobileNavItems so it's more in line with the other controls.

I am seeing an accessibility error in the dropdown items for an invalid aria-controls value on the large viewport size. Is this because the ID it's pointing at is not visible on the page?

HaSistrunk commented 2 years ago

Awesome, thanks @helrond

I am seeing an accessibility error in the dropdown items for an invalid aria-controls value on the large viewport size. Is this because the ID it's pointing at is not visible on the page?

Yes, I think you're right that that's what's happening since the 'ul' element with id="id-collections-menu" that the aria-controls is pointing to only appears via the storybook controls, so the accessibility checker doesn't know it exists until you toggle the showDropdownItems control. Once implemented it should be ok. I don't see that error on rockarch.org using the same accessibility checker (axe).