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

fix: Support false-only variant with fallback behaviour #159

Merged
merged 1 commit into from
Feb 12, 2024
Merged

fix: Support false-only variant with fallback behaviour #159

merged 1 commit into from
Feb 12, 2024

Conversation

lrholmes
Copy link
Contributor

Description

After upgrading to the latest version of tailwind-variants, we noticed a small UI regression where a component which specified only a false variant was continually using that, despite true being specifically passed in.

It looks like this occurs because of the logic to always fall back to the false variant if the initial value isn't found:

const value = variantObj[key] || variantObj["false"];

This PR addresses that by changing where the fallback happens. I've also added a couple of tests which cover the case that currently fails.

It's an odd case so hopefully this won't be impacting many other users! In our case, we actually realised that this API choice wasn't ideal and ended up implementing an alternative workaround. Still, the behaviour was very confusing to us for a moment and the fix seems quite simple, so decided to create a PR with the change.

Additional context


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Follow the Style Guide.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).

Copy link
Collaborator

@mskelton mskelton left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks for the PR!

@mskelton mskelton merged commit 183f21d into nextui-org:main Feb 12, 2024
4 checks passed
@lrholmes lrholmes deleted the fix-false-variant-case branch February 13, 2024 10:08
@p1pah p1pah mentioned this pull request Feb 15, 2024
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.

2 participants