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

refactor(sbb-tab): align with sbb-stepper #2744

Merged
merged 15 commits into from
Jun 14, 2024

Conversation

DavideMininni-Fincons
Copy link
Contributor

@DavideMininni-Fincons DavideMininni-Fincons commented Jun 12, 2024

Preflight Checklist

Issue

This PR Closes #2240 #1351

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

See Review Guidelines for more information what is checked during review process.

Changes

Changes in this pull request:

  • the sbb-tab-title has been renamed sbb-tab-label to align the naming with the stepper;
  • a new component named sbb-tab has been added; the content of a single tab within the tab group must be included in this new component (article, section and div are not supported anymore);
  • the sbb-tab has the logic to retrieve the related label
  • the didChange event now emits an object which contains the current (and previous, if available) tab index, tab label and tab content.

Browsers

I tested the build on the following browsers:

  • Firefox Desktop
  • Chrome Desktop
  • Edge Desktop
  • Safari Desktop
  • Chrome Mobile
  • Safari Mobile

Screen readers

I tested the build on the following browsers:

  • JAWS Firefox Desktop
  • JAWS Chrome Desktop
  • NVDA Firefox Desktop
  • NVDA Chrome Desktop
  • VoiceOver Safari Desktop
  • VoiceOver Chrome Desktop
  • VoiceOver Safari Mobile
  • Android Accessibility Suite Chrome Mobile

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

Does this introduce a breaking change?

  • Yes
  • No

BREAKING CHANGE

  • the sbb-tab-title has been renamed to sbb-tab-label;
  • the only supported tag for the tab content is the newly created sbb-tab; article, section and div are not supported anymore;
  • the didChange event on sbb-tab-group now includes an object which contains the currently selected tab index, sbb-tab-label and sbb-tab, and, if available, the previous ones.

Other information

…th-stepper

# Conflicts:
#	src/elements/tabs/tab-group/tab-group.e2e.ts
#	src/elements/tabs/tab-group/tab-group.spec.ts
#	src/elements/tabs/tab-label/__snapshots__/tab-label.spec.snap.js
#	src/elements/tabs/tab-label/__snapshots__/tab-title.snapshot.spec.snap.js
#	src/elements/tabs/tab-label/tab-title.snapshot.spec.ts
#	src/elements/tabs/tab-title/__snapshots__/tab-title.spec.snap.js
#	src/elements/tabs/tab-title/tab-title.spec.ts
@github-actions github-actions bot added the pr: peer review required A peer review is required for this pull request label Jun 12, 2024
@github-actions github-actions bot temporarily deployed to pr2744 June 12, 2024 08:49 Inactive
Copy link
Contributor

@jeripeierSBB jeripeierSBB left a comment

Choose a reason for hiding this comment

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

Nice work so far! I think also in terms of implementation there could be improvements to the previous version, but you covered the breaking changes, so probably for now it's should be ok :-).

src/elements/tabs/tab-group/readme.md Outdated Show resolved Hide resolved
src/elements/tabs/tab-group/readme.md Outdated Show resolved Hide resolved
src/elements/tabs/tab-group/tab-group.ts Show resolved Hide resolved
src/elements/tabs/tab-group/tab-group.ts Outdated Show resolved Hide resolved
src/elements/tabs/tab-group/tab-group.ts Show resolved Hide resolved
src/elements/tabs/tab/tab.ts Show resolved Hide resolved
Copy link
Contributor

@jeripeierSBB jeripeierSBB left a comment

Choose a reason for hiding this comment

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

LGTM, thank you

src/elements/tabs/tab-group/tab-group.ts Outdated Show resolved Hide resolved
@jeripeierSBB jeripeierSBB changed the title feat(sbb-tab): align with sbb-stepper refactor(sbb-tab): align with sbb-stepper Jun 14, 2024
@github-actions github-actions bot added the pr: lead review approved Pull request has been approved by a lead review label Jun 14, 2024
@DavideMininni-Fincons DavideMininni-Fincons merged commit 4305ca8 into main Jun 14, 2024
20 of 22 checks passed
@DavideMininni-Fincons DavideMininni-Fincons deleted the refactor/align-tab-with-stepper branch June 14, 2024 15:31
@github-actions github-actions bot added pr: peer review required A peer review is required for this pull request and removed pr: peer review required A peer review is required for this pull request labels Jun 14, 2024
This was referenced Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
diff-available pr: lead review approved Pull request has been approved by a lead review pr: peer review required A peer review is required for this pull request pr: visual review required preview-available
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactoring: align tab components with stepper components
2 participants