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

feat: Add consistent variant prop for design variant of buttons / chips #6472

Merged
merged 6 commits into from
Mar 6, 2025

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Jan 29, 2025

☑️ Resolves

Currently we need to wrap the native type prop of a button into nativeType of NcButton.
This is confusing and we should never use names for props that are already attributes of the native HTML element.

The solution is to rename type to variant.
To not make it breaking following is applied:

  1. variant prop is added replacing type
  2. nativeType is deprecated in favor of type
  3. type allows both, the native button type and the color variant
  • If a color variant is passed then the legacy behavior is applied
  • Otherwise it behaves like nativeType

The same is also applied for NcDialogButton and NcActions.
NcChip also referred to the NcButton for the type so this is also migrated the same way to make the wording variant consistent.

In a last step all components are refactored to use the new props instead.

🖼️ Screenshots

Nothing visually changed - if it does that's a regression 😨

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable
  • 3️⃣ Backport to next requested with a Vue 3 upgrade

@susnux susnux added enhancement New feature or request 3. to review Waiting for reviews feature: button labels Jan 29, 2025
@susnux susnux added this to the 8.23.0 milestone Jan 29, 2025
@Antreesy Antreesy modified the milestones: 8.23.0, 8.24.0 Feb 13, 2025
susnux added 6 commits March 1, 2025 13:40
Deprecate `type` for usage with the color variant.
Also deprecate `nativeType` in favor of `type` and `variant`.

Signed-off-by: Ferdinand Thiessen <[email protected]>
This is done to align with `NcButton`.

Signed-off-by: Ferdinand Thiessen <[email protected]>
To align with `NcButton`.

Signed-off-by: Ferdinand Thiessen <[email protected]>
…`type` prop

To align with `NcButton`.

Signed-off-by: Ferdinand Thiessen <[email protected]>
…e` prop

To align with `NcButton`.

Signed-off-by: Ferdinand Thiessen <[email protected]>
@susnux susnux requested a review from Antreesy March 1, 2025 12:40
Comment on lines +27 to +20
<!-- Before: -->
<NcButton type="primary" native-type="submit">Submit</NcButton>
<!-- After: -->
<NcButton type="submit" variant="primary">Submit</NcButton>
Copy link
Contributor

@ShGKme ShGKme Jan 29, 2025

Choose a reason for hiding this comment

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

To keep semantics - only prop names were renamed

Suggested change
<!-- Before: -->
<NcButton type="primary" native-type="submit">Submit</NcButton>
<!-- After: -->
<NcButton type="submit" variant="primary">Submit</NcButton>
<!-- Before: -->
<NcButton type="primary" native-type="submit">Submit</NcButton>
<!-- After: -->
<NcButton variant="primary" type="submit">Submit</NcButton>

@susnux susnux merged commit 85ba508 into master Mar 6, 2025
23 checks passed
@susnux susnux deleted the feat/variant branch March 6, 2025 09:58
@susnux
Copy link
Contributor Author

susnux commented Mar 6, 2025

/backport to next

* Accepted values: primary, secondary, tertiary, tertiary-no-background, tertiary-on-primary, error, warning, success.
*
* @default 'secondary'
* @since 8.23.0
Copy link
Contributor

Choose a reason for hiding this comment

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

I was wondering why that wasn't working 🙈
This is in 8.24.0 😁

Copy link
Contributor

Choose a reason for hiding this comment

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

@susnux ⚠️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement New feature or request feature: button
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants