epicmaxco / vuestic-ui

Free and Open Source UI Library for Vue 3 🤘
https://vuestic.dev
MIT License
3.33k stars 323 forks source link

feature: support configure css variables generation location #3466

Closed loieee closed 11 months ago

loieee commented 1 year ago

support configure css variables generation location

Description

closes #3442 support configure css variables generation location, root element(html) or new style tag.

Types of changes

loieee commented 12 months ago

This is look great.

We also need to handle it in nuxt/packages. So instead of htmlAttrs we need to use content in useHead. Otherwise both style tag and html attribute will be rendered.

No, html arribute is default. It just adds an optional configuration so that we can choose to transfer the color variables to a new style tag. I don't think that I can decide where the color variables are generated (this decision should be done the vuestic-ui's owner). so I just add new optional configuration for those who need it.

loieee commented 12 months ago

I got some screenshot for this.

  1. before, all color variables in the root (html) attributes.

Snipaste_2023-06-06_15-05-54

  1. when configuration styleTag is true. It's look tidy better.

Snipaste_2023-06-06_15-04-57

  1. and the color variable will be placed in the new generated style tag.

Snipaste_2023-06-06_15-05-25

m0ksem commented 12 months ago

I added some changes related to nuxt, but there is still a small bug.

We render style tag in nuxt using useHead, but in vuestic-ui we render the same style tag using our utility. This is why we have two similar style tags in nuxt application. I think we should be able to prevent rendering any kind of style tags in ui, so it can be handled in nuxt (or another ssr solution).

Will make a different issue for that. Looks awesome for SPA btw. Thanks!

m0ksem commented 12 months ago

@shiina7, would you like to write docs for this feature under Services/ColorConfig?

loieee commented 12 months ago

@shiina7, would you like to write docs for this feature under Services/ColorConfig?

@m0ksem sure, I'd love it! I will finish it in two days (before Thursday). Thank you for your additions for nuxt and merge request.

asvae commented 11 months ago

@shiina7 does it make any point to keep previous solution and not just keep yours? (and get rid of styleTag)

loieee commented 11 months ago

@shiina7 does it make any point to keep previous solution and not just keep yours? (and get rid of styleTag)

@asvae my first point is to expend it, not change. beacuse I think that I can't decide it, but you(author) can. If you think it's okay to just keep the new style tag, I'm glad to change it. But I think it's up to you to decide it. That's why added an option to configuration.

loieee commented 11 months ago

@shiina7 does it make any point to keep previous solution and not just keep yours? (and get rid of styleTag)

@asvae my first point is to expend it, not change. beacuse I think that I can't decide it, but you(author) can. If you think it's okay to just keep the new style tag, I'm glad to change it. But I think it's up to you to decide it. That's why added an option to configuration.

Perhap someone needs this option and we can keep, If we want keep new tag we can set the styleTag to true by default. BTW

m0ksem commented 11 months ago

I think we can remove previous solution. It was made asap and not documented, so we can just drop it.

loieee commented 11 months ago

I think we can remove previous solution. It was made asap and not documented, so we can just drop it.

Okay, got it. It's will be change it tomorrow.

loieee commented 11 months ago

I think we can remove previous solution. It was made asap and not documented, so we can just drop it.

Okay, got it. It's will be change it tomorrow.

@m0ksem @asvae I removed the styleTag option and only support generated css variables to style tag now. review it please, maybe it can be included in the next version.

asvae commented 11 months ago

It's actually great to have in context of 1.7.0, as if something would break - users would probably somewhat expect it :D.

Thanks @shiina7 🤗