kidonng / daisyui

A UnoCSS-compatible redistribution of daisyUI
https://npm.im/@kidonng/daisyui
MIT License
21 stars 2 forks source link

fix mask icon issue on chrome #3

Closed 524c closed 1 year ago

524c commented 1 year ago

Hello, congratulations on the project.

I created a port of it to run on nodejs, because in the current environment where I have unocss and daisyui we don't use deno. And while using this redistribution of daisyUI I noticed some issues and decided to contribute this PR to fix the icon mask.

In chrome, icon masks do not work correctly, as in the screenshot below. In Firefox and Safari the problem does not occur.

mask-star-issue chat-message-mask-issue

To resolve it, it was enough to include the autoprefixer plugin in postcss:

mask-star-after-autoprefixer chat-message-after-autoprefixer

// add postcss autoprefixer plugin

import nested from 'postcss-nested'
import autoprefixer from 'autoprefixer'
import stripIndent from 'strip-indent'
import themes from 'daisyui/src/colors/themes'
import functions from 'daisyui/src/colors/functions'
import {replacePrefix, replaceSlash, writeIndex} from './utils.ts'

const processor = postcss([nested, autoprefixer])

Another thing I noticed was that the root index.css was incremented on every run, so I'm removing it at the beginning of the script:

/* Root index */
if (existsSync('./index.css')) {
    Deno.removeSync('./index.css')
}
524c commented 1 year ago

Hey @kidonng,

Another equivalent way to fix the problem with icon masks is to compile daisyui and point as root "daisyui/dist" instead of "daisyui/src".

I looked at the fork of @philippedasilva-orizone, but I realized that using the compiled daisyui/dest is not ideal, and it affected, for example, the primary buttons in the mouse hove action. I believe that the ideal is to use the autoprefixer.

kidonng commented 1 year ago

I created a port of it to run on nodejs, because in the current environment where I have unocss and daisyui we don't use deno.

Oops, I didn't expect people to further customize the setup πŸ˜… The port should have been straightforward, but I still feel sorry for my niche (?) preference to have wasted other people's time.

Back to the issue. This looks like my oversight, but before adding autoprefixer, I need to ensure if the extra bytes are worth it for everyone.

Another thing I noticed was that the root index.css was incremented on every run, so I'm removing it at the beginning of the script:

Hmm, that should not happen because the code is specifically checking that (dir !== dirs[0]):

https://github.com/kidonng/daisyui/blob/3c8500085c1b8fcfc2c50806a5d697e72809a906/build.ts#L19-L23

524c commented 1 year ago

Hey @kidonng,

Congratulations on the project πŸ‘ About Node js it's just a preference of mine (I don't know the Deno well yet).

As for the autoprefixer, incorporated as a fix in a fork of your project, and it's been working fine for quite some time.

kidonng commented 1 year ago

Yup, the size changes are trivial (and most visible in mask icons as the inlined SVGs just doubled πŸ˜† glad that number isn't exploding thanks to https://github.com/saadeghi/daisyui/pull/1098)

More importantly, it isn't straightforward to use Autoprefixer (which requires PostCSS) alongside UnoCSS. So might as well do it here.

Thanks for you contribution!