Skip to content

Conversation

@TonyFresneau
Copy link
Contributor

@TonyFresneau TonyFresneau commented Jul 2, 2025

❓ Type of change

  • 🐞 Bug fix (a non-breaking change that fixes an issue)

πŸ“š Description

  • Fix issue where activeClass and inactiveClass props were not being combined with app.config variants
  • Ensure both app.config classes and component props are applied together

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.
  • I have tested the fix with various combinations of app.config and props
  • I have verified backward compatibility
  • The fix maintains the existing API without breaking changes

…iants

- Fix issue where activeClass and inactiveClass props were not being combined with app.config variants
- Ensure both app.config classes and component props are applied together
- Maintain backward compatibility with existing configurations
@pkg-pr-new
Copy link

pkg-pr-new bot commented Jul 2, 2025

npm i https://pkg.pr.new/@nuxt/ui@4446

commit: b5d1685

@TonyFresneau
Copy link
Contributor Author

TonyFresneau commented Jul 2, 2025

@benjamincanac

I updated the description, i mentionned the bad issue, this fix #4279

Main issue here was anything in variants in app.config doest not have any effect, for example

ui: {
    link: {
      variants: {
        active: {
          false: 'text-red-500',
        },
      },
    },
  },

doesn't work in current version 3.2.0

@TonyFresneau
Copy link
Contributor Author

@benjamincanac

I updated the description, i mentionned the bad issue, this fix #4279

Main issue here was anything in variants in app.config doest not have any effect, for example

ui: {
    link: {
      variants: {
        active: {
          false: 'text-red-500',
        },
      },
    },
  },

doesn't work in current version 3.2.0

Im not sure this wil fix #4279 btw, but it fix issues with ULink component variants that can't be customize with app.config

@benjamincanac
Copy link
Member

benjamincanac commented Jul 2, 2025

It doesn't seem related to #4279. Do you have a reproduction of what's not working?

@TonyFresneau
Copy link
Contributor Author

TonyFresneau commented Jul 3, 2025

It doesn't seem related to #4279. Do you have a reproduction of what's not working?

@benjamincanac

Hello my friend, there is a repro :

https://stackblitz.com/edit/sb1-wmy9hv9o

In app.config.ts

 ui: {
    link: {
      base: 'italic',  // work
      variants: {
        active: {
          false: '!text-red-500', // don't work
          true: '!text-2xl', // don't work
        },
      },
    },
  },

base: 'italic' -> work

 variants: {
  active: {
    false: '!text-red-500', // don't work
    true: '!text-2xl', // don't work
  },

-> don't work

In the screen below, "test" should be red right ?

image

Following the doc

image

@benjamincanac benjamincanac changed the title fix(ULink): merge activeClass/inactiveClass props with app.config variants fix(Link): merge active-class / inactive-class with app config Jul 3, 2025
@benjamincanac benjamincanac changed the title fix(Link): merge active-class / inactive-class with app config fix(Button/Link): merge active-class / inactive-class with app config Jul 3, 2025
Copy link
Member

@benjamincanac benjamincanac left a comment

Choose a reason for hiding this comment

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

I've updated your code to keep using defu which keep it simple and applied the same logic to the Button component. Let me know what you think!

@TonyFresneau
Copy link
Contributor Author

I've updated your code to keep using defu which keep it simple and applied the same logic to the Button component. Let me know what you think!

Good job for thinking of the Button, I didn't think to check the other components. MergeClasses function makes sense this way.

Ok for defu, I hesitated to remove it, I admit.

It'll make a great fix!

Bonne journΓ©e, Γ  plus !

@benjamincanac benjamincanac merged commit 9debce7 into nuxt:v3 Jul 3, 2025
6 checks passed
@benjamincanac benjamincanac added the v3 #1289 label Aug 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v3 #1289

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants