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

Update @material/web version and GM3 Checkbox component. #6696

Closed
techanvil opened this issue Mar 8, 2023 · 5 comments
Closed

Update @material/web version and GM3 Checkbox component. #6696

techanvil opened this issue Mar 8, 2023 · 5 comments
Labels
Exp: SP P1 Medium priority Type: Enhancement Improvement of an existing feature

Comments

@techanvil
Copy link
Collaborator

techanvil commented Mar 8, 2023

Feature Description

The @material/web package has published some more pre-release versions since we installed it. We should install the latest version (v1.0.0-pre.3 at the time of writing).

We should then update the GM3 Checkbox component to removing the label hack that adds a click handler to the label so the label can also trigger the checkbox, as this will no longer be necessary since @material/web now supports form-associated labels for checkboxes, which provide this behaviour as standard.

Note, it looks like we should not remove the aria-label, as it appears screen readers still need this as they won't pick up the label text via the form-associated label (see HTML Labels in the one-pager document). Besides which, adding this aria-label is not in the same hackish territory as the click handler and it's arguably adding some useful semantics anyway.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • @material/web should be updated to the latest version.
  • The GM3 Checkbox component should have its label click handler removed, while clicking on the label should still trigger the checkbox.

Implementation Brief

  • Update the @material/web NPM dependency to the latest version.
  • Remove the GM3 Checkbox label click handler.

Test Coverage

  • No new tests are needed.
  • Existing tests should continue to pass (note, the label click behaviour is covered in JS tests).

QA Brief

Changelog entry

  • Update Material 3 Checkbox component.
@techanvil techanvil added Exp: SP P1 Medium priority Type: Enhancement Improvement of an existing feature labels Mar 8, 2023
@eugene-manuilov eugene-manuilov self-assigned this Mar 22, 2023
@eugene-manuilov
Copy link
Collaborator

IB ✔️

@eugene-manuilov eugene-manuilov removed their assignment Mar 22, 2023
@techanvil
Copy link
Collaborator Author

Hmm, while working on the IB for #6652 I've noticed we may run into some complications with this issue.

Having updated @material/web to the latest version, this in turn bumps lit, and when I run the Checkbox tests, they fail due to an error TypeError: this.attachInternals is not a function.

attachInternals is not yet supported by JSDom: jsdom/jsdom#3444, so we may run into a wall here, hopefully there's a way around it.

Anyhow, just thought I'd put a preemptive flag here to raise awareness!

@techanvil techanvil self-assigned this May 30, 2023
@techanvil techanvil removed their assignment May 30, 2023
@tofumatt tofumatt assigned tofumatt and unassigned tofumatt May 31, 2023
@wpdarren wpdarren self-assigned this Jun 7, 2023
@wpdarren
Copy link
Collaborator

wpdarren commented Jun 7, 2023

QA Update: ⚠️

@techanvil On Storybook when I click on the default, complex label and checked checkboxes (or press space) nothing happens. Is that expected? 🤔 The interactive one does function as expected. It possibly is but want to double check. I am looking at this figma file as a comparison, is that the correct one?

@techanvil
Copy link
Collaborator Author

techanvil commented Jun 7, 2023

Hey @wpdarren, this is to be expected yes, the interactive one as per the name is the only one that is expected to change. These stories follow the same structure as the existing GM2 versions as seen here.

Also, yes, the Figma file you are looking at is the correct reference for these components.

@techanvil techanvil assigned wpdarren and unassigned wpdarren and techanvil Jun 7, 2023
@wpdarren
Copy link
Collaborator

wpdarren commented Jun 8, 2023

QA Update: ✅

Verified:

  • Compared the behaviour on the story to Figma and made sure that no regressions have occurred with the version change for GM3. I also looked at the ticket in the QAB and made sure that the steps there are recreated in this ticket.
gm3.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Exp: SP P1 Medium priority Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

5 participants