Skip to content

fix(ui-buttons): remove underline from disabled Button with href#2013

Merged
ToMESSKa merged 1 commit intomasterfrom
INSTUI-4540-remove-underline-from-disabled-base-button
Jun 13, 2025
Merged

fix(ui-buttons): remove underline from disabled Button with href#2013
ToMESSKa merged 1 commit intomasterfrom
INSTUI-4540-remove-underline-from-disabled-base-button

Conversation

@ToMESSKa
Copy link
Contributor

@ToMESSKa ToMESSKa commented Jun 5, 2025

INSTUI-4540

ISSUE:

  • BaseButton text has an underline when disabled and has a href prop

TEST PLAN:

  • test the code below, the buttons' text should not have an underline
<div>
  <BaseButton disabled href='#'>Click me</BaseButton>
  <Button disabled href='#'>Click me</Button>
</div>

@ToMESSKa ToMESSKa requested a review from Copilot June 5, 2025 11:24
@ToMESSKa ToMESSKa self-assigned this Jun 5, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR ensures that disabled Button components rendered as links do not display an underline.

  • Introduces an isEnabled flag to style logic
  • Applies full button styles only when enabled, and sets textDecoration: 'none' when disabled
  • Adds a test to verify that disabled links have no underline

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
packages/ui-buttons/src/BaseButton/styles.ts Added isEnabled state and made baseButton styles conditional on that flag
packages/ui-buttons/src/BaseButton/props.ts Extended BaseButtonStyleProps with a new isEnabled property
packages/ui-buttons/src/BaseButton/index.tsx Removed conditional CSS prop and always applies styles.baseButton
packages/ui-buttons/src/BaseButton/new-tests/BaseButton.test.tsx New test confirming disabled link Buttons have no underline

@github-actions
Copy link

github-actions bot commented Jun 5, 2025

PR Preview Action v1.6.1
Preview removed because the pull request was closed.
2025-06-13 10:10 UTC

@ToMESSKa ToMESSKa requested review from balzss and matyasf June 6, 2025 14:32
Copy link
Collaborator

@matyasf matyasf left a comment

Choose a reason for hiding this comment

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

see my comment

'&:hover > [class$=-baseButton__content]': colorVariants[color!].hover
}
: {
textDecoration: 'none'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add here these:

          label: 'baseButton',
          appearance: 'none',

Label is needed, so it's easier to identity what this is in the DOM (also unit tests might use it), appearance: none might prevent some custom CSS to override things here (although I could not override it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@matyasf I added them

@ToMESSKa ToMESSKa force-pushed the INSTUI-4540-remove-underline-from-disabled-base-button branch from 014f238 to 7118312 Compare June 10, 2025 13:27
@ToMESSKa ToMESSKa requested a review from matyasf June 10, 2025 13:39
@ToMESSKa ToMESSKa force-pushed the INSTUI-4540-remove-underline-from-disabled-base-button branch from 7118312 to 05f72d9 Compare June 13, 2025 07:32
@ToMESSKa ToMESSKa merged commit 90e8ce7 into master Jun 13, 2025
11 checks passed
@ToMESSKa ToMESSKa deleted the INSTUI-4540-remove-underline-from-disabled-base-button branch June 13, 2025 10:10
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.

3 participants