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

SFD-181: Implement ARIA tabs patterns #133

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

jantoun-scottlogic
Copy link
Contributor

@jantoun-scottlogic jantoun-scottlogic commented Oct 3, 2024

<SFD-181> Jira ticket

Description of Change

Implements the ARIA tabs pattern with automatic activation.

Include any relevant screenshots of visual changes.

Checklist

  • tests are updated or added

Notes

  • I haven't added a label describing the tabs for now but that's something we could also add. I'm not sure what the best label would be for the input form/assumptions and limitations tabs.

@jantoun-scottlogic jantoun-scottlogic force-pushed the SFD-181-implement-aria-tabs-patterns branch from 9233057 to ff72388 Compare October 3, 2024 14:50
@jantoun-scottlogic jantoun-scottlogic force-pushed the SFD-181-implement-aria-tabs-patterns branch from 8e4f19b to eddf8ae Compare October 3, 2024 14:59
@jantoun-scottlogic jantoun-scottlogic force-pushed the SFD-181-implement-aria-tabs-patterns branch from bb320bd to 4aa19b6 Compare October 3, 2024 15:37
@jantoun-scottlogic jantoun-scottlogic changed the title SFD-181: Implement aria tabs patterns SFD-181: Implement ARIA tabs patterns Oct 4, 2024
This no longer seems to be needed after refactoring the tab panels.
@@ -92,7 +92,7 @@ def test_example(page: Page) -> None:
expect(page.get_by_text("Clearly there is a large")).to_be_visible()
expect(page.get_by_text("The estimated kWh of cloud")).to_be_visible()
expect(page.get_by_role("heading", name="Downstream Emissions")).to_be_visible()
expect(page.get_by_text("At present we focus on the")).to_be_visible()
expect(page.get_by_text("To do this we have collated")).to_be_visible()
Copy link
Contributor Author

@jantoun-scottlogic jantoun-scottlogic Oct 4, 2024

Choose a reason for hiding this comment

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

Now that the panels are hidden when not selected rather than being removed from the DOM there are 2 elements in the DOM with the old text so I've used the start of the second sentence in the section instead.

(ngSubmit)="handleSubmit()"
[formGroup]="estimatorForm"
class="tce-w-full tce-flex tce-flex-col tce-gap-6"
tabindex="0">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this should be in the tab sequence now that it's in a tabpanel. The other tabpanels are already behaving that way so I've added it here rather than on the actual tabpanel element since that was causing other panels to appear twice in the tab sequence.

@@ -11,7 +11,7 @@ <h1 class="tce-text-3xl tce-mb-6">Technology Carbon Estimator</h1>
(formReset)="handleFormReset()"></carbon-estimator-form>
</tab-item>
<tab-item [title]="'Assumptions and Limitations'">
<assumptions-and-limitation aria-live="polite"></assumptions-and-limitation>
Copy link
Contributor Author

@jantoun-scottlogic jantoun-scottlogic Oct 4, 2024

Choose a reason for hiding this comment

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

I originally removed this when changing some other aria-live values but having reverted those changes and rechecked, I don't think this is changing the behaviour now although that could just be my screen reader settings. I've left it off for now since other elements placed directly inside the tab-item components do not have this set.

@jantoun-scottlogic jantoun-scottlogic marked this pull request as ready for review October 4, 2024 08:52
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.

1 participant