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

[Bug]: Some components need whitespace-nowrap #199

Closed
anonimusprogramus opened this issue Dec 2, 2023 · 11 comments
Closed

[Bug]: Some components need whitespace-nowrap #199

anonimusprogramus opened this issue Dec 2, 2023 · 11 comments
Assignees
Labels
bug Something isn't working

Comments

@anonimusprogramus
Copy link
Contributor

Environment

Developement/Production OS: Linux Mint 20.1 Cinnamon
Node version: v18.16.0
Package manager: [email protected]
Radix Vue version: 1.1.1
Shadcn Vue version: 0.8.2
Vue version: 3.3.7
Nuxt version: 3.8.0
Nuxt mode: universal
Nuxt target: client server
CSS framework: [email protected]
Client OS: Linux Mint 20.1 Cinnamon
Browser: Brave 1.60

Link to minimal reproduction

https://www.shadcn-vue.com/themes

Steps to reproduce

  1. Open themes page
  2. Resize to xl or md screen size
  3. Reload the page
  4. For miliseconds you'll see Button's title "Copy code" being wrapped

2023-12-01 10-52-14

Describe the bug

Title having spaces may be wrapped because of missing whitespace-nowrap class.

In shadcn, following (new-york) components have whitespace-nowrap:

  • button
  • select
  • tabs

Meanwhile, in shadcn-vue only these:

  • table
  • tabs

2023-12-02 11-04-57

2023-12-02 11-05-12

Expected behavior

No wrap

Conext & Screenshots (if applicable)

No response

@anonimusprogramus anonimusprogramus added the bug Something isn't working label Dec 2, 2023
@sadeghbarati
Copy link
Collaborator

sadeghbarati commented Dec 2, 2023

@anonimusprogramus
Copy link
Contributor Author

Agree about that layout shift.

I looked into shadcn, looks like thay had the same problem and fixed it on Oct 21 by adding whitespace-nowrap

If we're gonna fix this, should button.json be fixed too?

@sadeghbarati
Copy link
Collaborator

sadeghbarati commented Dec 2, 2023

looked into shadcn-ui

  • button
  • select
  • tab

has whitespace-nowrap thanks for report

PR welcomed if you have time 🙏


If you start doing it please refactor the button to use VariantProps from class-variance-authority instead of NonNullable<Parameters<typeof buttonVariants>

@adirizky54
Copy link

@sadeghbarati I'm agree with you to use VariantProps instead of NonNullable<Parameters<typeof buttonVariants>, but
Type version of defineProps isn't smart enough to resolve more complicated types in Vue, and you will getting error like this.

Screenshot 2023-12-03 at 16 42 40

@sadeghbarati
Copy link
Collaborator

sadeghbarati commented Dec 3, 2023

Ow man you right 😭, but how Svelte example support complex types?

export type VariantProps<Component extends (...args: any) => any> = Omit<OmitUndefined<Parameters<Component>[0]>, "class" | "className">;

Looks like we have to use it this way before complex types support

https://github.com/joe-bell/cva/blob/main/examples/vue/src/components/Button.vue

@adirizky54
Copy link

@hrynevychroman
Copy link
Collaborator

Started working on this 🙂

@hrynevychroman
Copy link
Collaborator

@sadeghbarati Hello 👋

Have one question about it, I don't think that it is great to use whitespace-nowrap on Select for example, because if selected text will be wider it will overlap 👇

image

So my solution is to change Select from flex to grid, so we can truncate selected text, what do you think? 👇

image

@hrynevychroman
Copy link
Collaborator

Please Assign me for this Issue, I already change Button with refactor VariantProps ❤️

@hrynevychroman
Copy link
Collaborator

One more question, sorry, I need to make PR when everything will be fixed, or better do it now and push new commits?

Working Branch: https://github.com/romanhrynevych/shadcn-vue/tree/199-bug-whitespace-nowrap

@sadeghbarati
Copy link
Collaborator

sadeghbarati commented Jan 11, 2024

It's up to you, PR Title is important

of course, individual commit messages are important but not as much as the first commit and PR Title

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants