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.

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

Change Safari 10.1 to Safari 11 in Grade C browser example #5082

Merged
merged 1 commit into from
Jul 1, 2024

Conversation

querkmachine
Copy link
Member

@querkmachine querkmachine commented Jun 17, 2024

The summary of browser grades include Safari 10.1 (macOS) and 10.3 (iOS) as examples of Grade C browsers.

However in the Grade C section of the document it's mentioned that Safari 10.1 will not run our JavaScript as (although it technically supports type="module") it fails the prerequisite support tests present in Frontend. That section also (confusingly) lists Safari 11 (macOS) as being the minimum for Grade C, not 10.1.

This is confusing, as different parts of the document use different Safari versions as the minimum supported version for Grade C. It also does not mention that Safari 10.3 (iOS) is subject to the same issues as Safari 10.1 (macOS).

Changes

  • Update the summary at the top of the document to list Safari 11 as the minimum supported Grade C version.
    • Although Safari 11 isn't the first version to support modules, it is the earliest version that doesn't fail Frontend's support checks, which is the information most people viewing the document will actually be looking for.
  • Likewise, updates the Grade C section to list Safari 11 as the minimum supported version.
  • Updates the Grade C footnote to clarify that Safari 10.1 (macOS) and Safari 10.3 (iOS) both support modules, but not nomodule, and consequentially will not run any Frontend component JavaScript.

Thoughts

Are there other browsers that support modules but not nomodule? Maybe there's room for a Grade D in here... 🤔

@querkmachine querkmachine requested a review from a team June 17, 2024 10:08
@querkmachine querkmachine self-assigned this Jun 17, 2024
Copy link

github-actions bot commented Jun 17, 2024

📋 Stats

File sizes

File Size
dist/govuk-frontend-development.min.css 113.34 KiB
dist/govuk-frontend-development.min.js 41.88 KiB
packages/govuk-frontend/dist/govuk/all.bundle.js 87.42 KiB
packages/govuk-frontend/dist/govuk/all.bundle.mjs 82.11 KiB
packages/govuk-frontend/dist/govuk/all.mjs 981 B
packages/govuk-frontend/dist/govuk/govuk-frontend-component.mjs 359 B
packages/govuk-frontend/dist/govuk/govuk-frontend.min.css 113.33 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend.min.js 41.87 KiB
packages/govuk-frontend/dist/govuk/i18n.mjs 5.55 KiB
packages/govuk-frontend/dist/govuk/init.mjs 4.86 KiB

Modules

File Size (bundled) Size (minified)
all.mjs 79.24 KiB 39.84 KiB
accordion.mjs 23.5 KiB 12.39 KiB
button.mjs 5.98 KiB 2.69 KiB
character-count.mjs 22.4 KiB 9.92 KiB
checkboxes.mjs 5.83 KiB 2.83 KiB
error-summary.mjs 7.89 KiB 3.46 KiB
exit-this-page.mjs 17.1 KiB 9.26 KiB
header.mjs 4.46 KiB 2.6 KiB
notification-banner.mjs 6.26 KiB 2.62 KiB
password-input.mjs 15.15 KiB 7.25 KiB
radios.mjs 4.83 KiB 2.38 KiB
skip-link.mjs 4.39 KiB 2.18 KiB
tabs.mjs 10.13 KiB 6.11 KiB

View stats and visualisations on the review app


Action run for 1099d1c

@domoscargin
Copy link
Contributor

domoscargin commented Jun 17, 2024

The problem is we've defined Grade C as everything not in A or B that supports <script type="module">, which includes Safari 10.1, even if we do the noModule check before it can do anything.

Because the type attribute is present but has a value older browsers (Grade X) don't understand, they should not process the <script> block. Safari 10.1, however, will process the <script> block, and thus we have to ensure our script runs and doesn't break anything.

@querkmachine
Copy link
Member Author

Truuuuue. I suppose Safari 10.1 is just an annoying special case. From our perspective, it's JS enabled but subsequently has everything turned off (including "necessary enhancements"), whereas from a user perspective it just doesn't appear to run JS at all.

I still think the documentation is a little confusing as-is, as the Grade C section appears to list Safari 11 as the minimum version that's in Grade C, contrary to the summary at the top of the page. It tripped me up, at least!

@domoscargin
Copy link
Contributor

I wonder if we should add some kind of link to documentation in this error message:

: 'GOV.UK Frontend is not supported in this browser'

But not sure how many people who get into this specific case will be checking the console.

As for the documentation, maybe we can add a "special cases" heading or something in the Grade C section so that it's clearer there's some niche case?

@romaricpascal
Copy link
Member

Thinking the issue might be with the definition of what we're supporting. We document that we run in browsers that support <script type="module"> where actually, we make the cut at browser that support <script nomodule>. I think that if we clarify the feature we base the support on, we could clarify to Safari 11 without ambiguity.

Maybe with either:

  1. 'browsers that support <script nomodule>', but that's a bit cryptic
  2. 'browsers that fully support ES6 Modules'

@domoscargin
Copy link
Contributor

domoscargin commented Jun 17, 2024

I like the second option for simplicity. Then we could simplify the Grade C section documentation as well:

Grade C

This grade covers browsers not in Grade A or B which fully support <script type="module">. These are:

  • Chrome 61+
  • Edge 16-18
  • Edge 79+
  • Safari 11
  • Firefox 60+
  • Opera 48+
  • Samsung Internet 8.2+

@querkmachine
Copy link
Member Author

querkmachine commented Jun 25, 2024

Coincidentally, this was brought up on support today. It was questioned why the Frontend Docs currently state Safari 10.3 as the minimum supported version when Safari 10.3 appears to behave the same as 10.1 — JS is loaded but no components are initialised. (Indeed, Safari 10.3 also does not appear to support nomodule.)

In my mind, bumping the 'headline' version of Safari supported in Grade C to 11.0 would make the documentation easier to understand at a glance. 10.1–10.3 can be in the footnote.

I'll update my suggested edits shortly.

The summary of browser grades include Safari 10.1 (macOS) and 10.3 (iOS) as examples of Grade C browsers.

However in the Grade C section of the document it's mentioned that Safari 10.1 will not run our JavaScript as (although it technically supports `type="module"`) it fails the prerequisite support tests present in Frontend. That section lists Safari 11 (macOS) as being the minimum for Grade C instead.

This is confusing, as different parts of the document use different Safari versions as the minimum supported version for Grade C. It also does not mention that Safari 10.3 (iOS) is subject to the same issues as Safari 10.1 (macOS).

This commit updates the summary at the top of the document and in the Grade C section to use Safari 11 as the minimum supported version, as this is what appears to be the case to a person quickly glancing at the document in reference. The Grade C section has been updated to make it clear that, while Safari 10.1 (macOS) and Safari 10.3 (iOS) are technically included, they do not actually initialise any of Frontend's components due to failing other prerequisites outside of having support for modules.
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-5082 June 25, 2024 14:54 Inactive
Copy link
Member

@romaricpascal romaricpascal left a comment

Choose a reason for hiding this comment

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

Sounds good to merge for me. Let's make sure we co-ordinate with the merge of the same change on the frontend docs so we don't have two different wordings in different places.

@querkmachine querkmachine merged commit 51cf8a3 into main Jul 1, 2024
51 checks passed
@querkmachine querkmachine deleted the querkmachine-patch-1 branch July 1, 2024 10:23
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.

4 participants