Skip to content
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.

Sign up for GitHub

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

Style details in older browsers #3758

Merged
merged 3 commits into from
Jun 20, 2023
Merged

Style details in older browsers #3758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
da96d31
Wrap everything in support query
domoscargin Jun 6, 2023
a1cd5d0
Style to look like inset text on older browsers
domoscargin Jun 12, 2023
1084233
Default to native styling
domoscargin Jun 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 96 additions & 51 deletions packages/govuk-frontend/src/govuk/components/details/_index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,78 +11,123 @@
// Make the focus outline shrink-wrap the text content of the summary
display: inline-block;

// Absolutely position the marker against this element
position: relative;

margin-bottom: govuk-spacing(1);
Copy link
Member

@querkmachine querkmachine Jun 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This margin is being ignored in EdgeHTML Edge. It seems like you need to maintain the display: inline-block for this to be applied correctly.

Copy link
Contributor Author

@domoscargin domoscargin Jun 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm, I can't reproduce this via Browserstack in Edge 15 - 18. What are you getting?
Details component in Edge 18, showing element styles

Details component in Edge 18, showing computed styles

Copy link
Member

Choose a reason for hiding this comment

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

The style appears in the inspector's style and computed tabs, but if you hover over the element so that the box model is highlighted on the page you can see that the margin-bottom value hasn't actually been used, even though it's not crossed out.

The difference can also be seen if you do a side-by-side comparison with the existing review app—the border of the <details> contents has moved up by 5 pixels.

image

I'm not sure why EdgeHTML devtools says the style is being used when it actually isn't, but I'm gonna chalk that up to EdgeHTML devtools maybe not being amazing. 😝

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aah, nice spot! My basic trust in devtools is forever tainted.

Copy link
Contributor Author

@domoscargin domoscargin Jun 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It seems like you need to maintain the display: inline-block for this to be applied correctly.

Done. Looking good so far.
Details component in older browser

}

// Allow for absolutely positioned marker and align with disclosed text
padding-left: govuk-spacing(4) + $govuk-border-width;

// Style the summary to look like a link...
color: $govuk-link-colour;
cursor: pointer;

&:hover {
color: $govuk-link-hover-colour;
.govuk-details__summary-text {
> :first-child {
margin-top: 0;
}

&:focus {
@include govuk-focused-text;
> :only-child,
> :last-child {
margin-bottom: 0;
}
}

// ...but only underline the text, not the arrow
.govuk-details__summary-text {
@include govuk-link-decoration;
.govuk-details__text {
padding-top: govuk-spacing(3);
padding-bottom: govuk-spacing(3);
padding-left: govuk-spacing(4);
}

.govuk-details__summary:hover .govuk-details__summary-text {
@include govuk-link-hover-decoration;
.govuk-details__text p {
margin-top: 0;
margin-bottom: govuk-spacing(4);
}

// Remove the underline when focussed to avoid duplicate borders
.govuk-details__summary:focus .govuk-details__summary-text {
text-decoration: none;
.govuk-details__text > :last-child {
margin-bottom: 0;
}

// Remove the default details marker so we can style our own consistently and
// ensure it displays in Firefox (see implementation.md for details)
.govuk-details__summary::-webkit-details-marker {
display: none;
}
// Hack to target IE8 - IE11 (and REALLY old Firefox)
// These browsers don't support the details element, so fall back to looking
// like inset text
@media screen\0 {
.govuk-details {
border-left: $govuk-border-width-wide solid $govuk-border-colour;
}

.govuk-details__summary {
margin-top: govuk-spacing(3);
}

// Append our own open / closed marker using a pseudo-element
.govuk-details__summary::before {
content: "";
position: absolute;
.govuk-details__summary-text {
@include govuk-typography-weight-bold;
@include govuk-responsive-margin(4, "bottom");
padding-left: govuk-spacing(4);
}
}

top: -1px;
bottom: 0;
left: 0;
// We wrap styles for newer browsers in a feature query, which is ignored by
// older browsers, which always expand the details element.
//
// Additionally, -ms-ime-align is only supported by Edge 12 - 18
//
// This ensures we don't use these styles in browsers which:
// - support ES6 modules but not the <details> element (Edge 16 - 18)
// - do not support ES6 modules or the <details> element (eg, IE8+)
@supports not (-ms-ime-align: auto) {
.govuk-details__summary {

// Absolutely position the marker against this element
position: relative;

// Allow for absolutely positioned marker and align with disclosed text
padding-left: govuk-spacing(4) + $govuk-border-width;

// Style the summary to look like a link...
color: $govuk-link-colour;
cursor: pointer;

&:hover {
color: $govuk-link-hover-colour;
}

&:focus {
@include govuk-focused-text;
}
}
// ...but only underline the text, not the arrow
.govuk-details__summary-text {
@include govuk-link-decoration;
}

margin: auto;
.govuk-details__summary:hover .govuk-details__summary-text {
@include govuk-link-hover-decoration;
}

@include govuk-shape-arrow($direction: right, $base: 14px);
// Remove the underline when focussed to avoid duplicate borders
.govuk-details__summary:focus .govuk-details__summary-text {
text-decoration: none;
}

.govuk-details[open] > & {
@include govuk-shape-arrow($direction: down, $base: 14px);
// Remove the default details marker so we can style our own consistently and
// ensure it displays in Firefox (see implementation.md for details)
.govuk-details__summary::-webkit-details-marker {
display: none;
}
}

.govuk-details__text {
padding-top: govuk-spacing(3);
padding-bottom: govuk-spacing(3);
padding-left: govuk-spacing(4);
border-left: $govuk-border-width solid $govuk-border-colour;
}
// Append our own open / closed marker using a pseudo-element
.govuk-details__summary::before {
content: "";
position: absolute;

.govuk-details__text p {
margin-top: 0;
margin-bottom: govuk-spacing(4);
}
top: -1px;
bottom: 0;
left: 0;

.govuk-details__text > :last-child {
margin-bottom: 0;
margin: auto;

@include govuk-shape-arrow($direction: right, $base: 14px);

.govuk-details[open] > & {
@include govuk-shape-arrow($direction: down, $base: 14px);
}
}

.govuk-details__text {
border-left: $govuk-border-width solid $govuk-border-colour;
}
}
}
44 changes: 44 additions & 0 deletions packages/govuk-frontend/src/govuk/components/details/implementation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,49 @@
## Implementation notes

### Styling in older browsers
Copy link
Contributor

Choose a reason for hiding this comment

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

😍


#### Browsers with support for `type=module`, `<details>` and feature queries

https://browsersl.ist/#q=supports+details+and+supports+css-featurequeries+and+supports+es6-module&region=GB

Previously, we provided a polyfill for older browsers which do not support the
`<details>` component. Since most browsers now DO support `<details>`, we've
removed that polyfill, and we need to make sure browsers which don't support
`<details>` don't use any styles that make them look interactable.

By wrapping these styles in a feature query, we can capture the vast majority of
browsers which have full support.

#### Browsers that support `type=module` and feature queries but not `<details>`

https://browsersl.ist/#q=supports+es6-module+and+not+supports+details+and+supports+css-featurequeries&region=GB

This only affects Edge 16 - 18. We filter these out of the support query by
checking for `-ms-ime-align: auto`, which isn't supported by any other browsers.

#### Browsers that support `<details>` but not `type=module` or feature queries

https://browsersl.ist/#q=supports+details+and+not+supports+css-featurequeries+and+not+supports+es6-module&region=GB

These will default to their native styling of the `<details>` element, with a
few of our spacing and font styles.

#### Browsers which don't support `<details>`, `type=module` or feature queries

https://browsersl.ist/#q=%3E0.0000001%25+and+not+supports+details+and+not+supports+es6-module+and+not+supports+css-featurequeries&region=GB

This is largely IE 8 - 11.

We can account for IE by styling them like inset text, via a `@media screen\0`
hack that no other browser supports.

#### Browsers that support feature queries, but not `<details>` or `type=module`

https://browsersl.ist/#q=supports+css-featurequeries+and+not+supports+details+and+not+supports+es6-module&region=GB

This is the only gap, as these browsers will get styled to look interactable,
even though they aren't. This is largely Opera Mini.

### Marker styling

Firefox uses display: list-item to show the arrow marker for the summary
Expand Down