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

Jump to bottom

style(sbb-button): button disabled state #2651

Merged
merged 8 commits into from
May 28, 2024
Merged

style(sbb-button): button disabled state #2651

merged 8 commits into from
May 28, 2024

Conversation

DavideMininni-Fincons
Copy link
Contributor

@DavideMininni-Fincons DavideMininni-Fincons commented May 13, 2024 •
edited
Loading

Preflight Checklist

Issue

This PR Closes #2636

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:

  • new disabled state

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

Other information

@github-actions github-actions bot temporarily deployed to preview-pr2651 May 13, 2024 07:44 Inactive
@github-actions github-actions bot temporarily deployed to preview-pr2651 May 13, 2024 07:58 Inactive
@DavideMininni-Fincons DavideMininni-Fincons changed the title style(sbb-button): button disabled state [WIP] style(sbb-button): button disabled state May 13, 2024
@github-actions github-actions bot added the pr: peer review required A peer review is required for this pull request label May 13, 2024
@jeripeierSBB jeripeierSBB added the pr: ux review required A ux review is required for this pull request label May 14, 2024
@github-actions github-actions bot temporarily deployed to preview-pr2651 May 14, 2024 07:13 Inactive
@mcilurzo
Copy link
Contributor

ux-review: the border color seems wrong. it sould be "smoke" in the default and negative variant.

@github-actions github-actions bot temporarily deployed to pr2651 May 15, 2024 15:53 Inactive
TomMenga
TomMenga approved these changes May 23, 2024
View reviewed changes
Copy link
Contributor

@TomMenga TomMenga left a comment

Choose a reason for hiding this comment

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

Just a small thing.
The rest is lgtm 👍

src/components/button/common/button-common.scss Outdated Show resolved Hide resolved
@github-actions github-actions bot added pr: peer review approved Pull request has been approved by a peer review and removed pr: peer review required A peer review is required for this pull request labels May 23, 2024
@mcilurzo
Copy link
Contributor

ux review: the border of the disabled state should be 1px

@github-actions github-actions bot temporarily deployed to pr2651 May 24, 2024 07:45 Inactive
@github-actions github-actions bot added pr: ux review approved Pull request has been approved by a ux review and removed pr: ux review required A ux review is required for this pull request labels May 24, 2024
@mcilurzo mcilurzo added ux approved Issue is approved by UX and removed pr: ux review approved Pull request has been approved by a ux review labels May 24, 2024
jeripeierSBB
jeripeierSBB approved these changes May 28, 2024
View reviewed changes
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

@github-actions github-actions bot added the pr: lead review approved Pull request has been approved by a lead review label May 28, 2024
@DavideMininni-Fincons DavideMininni-Fincons merged commit 20a2aed into main May 28, 2024
25 of 26 checks passed
@DavideMininni-Fincons DavideMininni-Fincons deleted the feat/button-disabled-state branch May 28, 2024 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcilurzo mcilurzo mcilurzo approved these changes

@jeripeierSBB jeripeierSBB jeripeierSBB approved these changes

@TomMenga TomMenga TomMenga approved these changes

@kyubisation kyubisation Awaiting requested review from kyubisation kyubisation is a code owner

@MarioCastigliano MarioCastigliano Awaiting requested review from MarioCastigliano

@dauriamarco dauriamarco Awaiting requested review from dauriamarco

Assignees
No one assigned
Labels
diff-available pr: lead review approved Pull request has been approved by a lead review pr: peer review approved Pull request has been approved by a peer review pr: visual review required preview-available ux approved Issue is approved by UX
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

story(sbb-button): adapt new disabled design
5 participants
@DavideMininni-Fincons @mcilurzo @TomMenga @jeripeierSBB @kyubisation