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

remove duplicate: xxl and 2xl is the same thing #1633

Closed
razbakov opened this issue Dec 13, 2024 · 7 comments · Fixed by #1632
Closed

remove duplicate: xxl and 2xl is the same thing #1633

razbakov opened this issue Dec 13, 2024 · 7 comments · Fixed by #1632
Labels
bug Something isn't working documentation Improvements or additions to documentation help wanted Extra attention is needed

Comments

@razbakov
Copy link
Contributor

There is a contradiction in configuration screens.

{
  'xs': 320,
  'sm': 640,
  'md': 768,
  'lg': 1024,
  'xl': 1280,
  'xxl': 1536,
  '2xl': 1536
}

We share the same naming and sizes as Tailwind CSS and we added the xs property.

Either this message should be adjusted or the extra property should be removed.

There are 2 ways to name sizes:

  • xl
  • xxl
  • xxxl

Or:

  • xl
  • 2xl
  • 3xl

Counting letters is more difficult than reading the number.
And xxl duplicates the value of 2xl.

@Baroshem
Copy link
Collaborator

I see. Thanks for clarification!

Then I think it makes sense but I wonder if it wouldn't be a breaking change? If someone was using current xxl size? @danielroe what do you think?

@Baroshem Baroshem added bug Something isn't working documentation Improvements or additions to documentation help wanted Extra attention is needed labels Dec 13, 2024
@danielroe
Copy link
Member

danielroe commented Dec 13, 2024

I think having an extra screen property is useful for backwards compatibility. (we can remove in next major)

I don't think it harms anything to keep it, though we can remove it from the docs if it is there. It's a superset of Tailwind.

@Baroshem
Copy link
Collaborator

Ok so let's maybe approach it this way:

  1. Let's leave the option in the src/module.ts for now but make it deprecated with a note that it will be removed in the next major.
  2. Remove this deprecated option from the docs already.

@razbakov could you modify your PR with that? :)

@razbakov
Copy link
Contributor Author

@Baroshem sure, can you show me how to make it deprecated?

@Baroshem
Copy link
Collaborator

Baroshem commented Dec 16, 2024

As this is an object and not a type, maybe we could add a note in the documentation instead of in the TS code?

Something like https://nuxt-security.vercel.app/utils/remove-console-loggers#alternative-method-deprecated

@razbakov
Copy link
Contributor Author

@danielroe the point 2 is not done

@danielroe
Copy link
Member

I think let's not deprecate it for now. It seems harmless.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants