Skip to content

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

Merged
susnux merged 6 commits into
masterfrom
feat/variant
Mar 6, 2025
Merged

feat: Add consistent variant prop for design variant of buttons / chips#6472
susnux merged 6 commits into
masterfrom
feat/variant

Conversation

@susnux

@susnux susnux commented Jan 29, 2025

Copy link
Copy Markdown
Contributor

☑️ 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
Comment thread src/components/NcActions/NcActions.vue Outdated
Comment thread src/components/NcDialogButton/NcDialogButton.vue Outdated
@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 <opensource@fthiessen.de>
This is done to align with `NcButton`.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
To align with `NcButton`.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
…`type` prop

To align with `NcButton`.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
…e` prop

To align with `NcButton`.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux requested a review from Antreesy March 1, 2025 12:40
Comment thread CHANGELOG.md
Comment on lines +27 to +20
<!-- Before: -->
<NcButton type="primary" native-type="submit">Submit</NcButton>
<!-- After: -->
<NcButton type="submit" variant="primary">Submit</NcButton>

@ShGKme ShGKme Jan 29, 2025

Copy link
Copy Markdown
Contributor

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
@susnux susnux deleted the feat/variant branch March 6, 2025 09:58
@susnux

susnux commented Mar 6, 2025

Copy link
Copy Markdown
Contributor Author

/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
Copy Markdown
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
Copy Markdown
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