search

alphagov / govuk_elements

❗️GOV.UK Elements is deprecated, and will only receive major bug fixes and security patches.
https://govuk-elements.herokuapp.com/
MIT License
227 stars 90 forks source link

Fix summary outline shrink-wrapping in Firefox #554

Closed robinwhittleton closed 6 years ago

robinwhittleton commented 7 years ago

The styling for <summary> on GOV.UK currently expects the focus ring to shrinkwrap the text. This works fine in Chrome and Safari, but fails in Firefox (see https://github.com/alphagov/govuk_elements/issues/553).

This removes the old workaround to get the arrows to show in Firefox. It then reimplements the arrows for everyone using counters. There is a potential problem that browsers that understand counter but not <details> might get another arrow in addition to the one generated by the polyfill. As luck would have it though, the list styles disclosure-open and disclosure-closed seem to have been invented for the details element, so the versions of Edge and IE that I tried this fix in don’t recognise the ::before/::after content rules and don’t show them.

Tested in latest Firefox / Chrome / Safari / Edge and IE11. Has not been tested on mobile, other browsers or with assistive tech.

What type of change is it?

kr8n3r commented 7 years ago

Screenshots from Firefox: Before: image

After: image

kr8n3r commented 7 years ago

There's 3 ways we can approach this 1) we have this slightly smaller arrow in Firefox (we can adjust padding) and have Safari and chrome use details-marker (plus polyfill for other browsers, that use \u25bc and \u25ba)

2) forcefully hide details-marker for Safari and Chrome and make them use \25B6 so that all browsers, including polyfill will the same icon. Behaviour of it is strange, it pushes text right, see here https://developer.mozilla.org/en-US/docs/Web/HTML/Element/details

3) replace details with button + javascript (we already have js polyfill for the element) Question here is, is details the best way to display disclosure content? Does it have better browser accessibility built in than button + content combo? I would assume so http://accessibleculture.org/articles/2012/03/screen-readers-and-details-summary/

Input here welcome @selfthinker @edwardhorsford

edwardhorsford commented 7 years ago

I don't feel able to comment too much on the options other than that we wouldn't want the text to move when opening and closing the item.

kr8n3r commented 6 years ago

Hi all, in Frontend we've taken a slightly different approach https://github.com/alphagov/govuk-frontend/pull/372

robinwhittleton commented 6 years ago

I’m happy for this to be closed then.

36degrees commented 6 years ago

Superseded by #611, which mirrors the approach we've taken with GOV.UK Frontend.