Date picker widget unreadable in modal in light mode #1074

I think #1070 merge has caused issues in light mode.

Styles changed in #1070 need moved into a nested &.dark {...} selector and the original light mode styles returned?

(Not every change in the dark mode is a simple switch, text in particular.)

EDIT: this is only happening when using the date widget in the modal view!

44 KiB

You are right. Was not very well tested by me.

This seems to be related to --text not being defined. I think this is a variable that should be defined from bulma-css-vars?

Put up a PR with a fix: #1079

This seems to be related to --text not being defined. I think this is a variable that should be defined from bulma-css-vars?

So that's --text and --link that were missing...I can't help but think something deeper is wrong.

I swear I would have noticed before now if --text was missing. I'm wondering if I clobbered some of the sass imports @dpschen provided in PRs to my branch during an upstream merge.

There are quite a lot of undefined custom properties. Not just those two.

Both --text and --link are set in bulma-css-variables/sass/utilities/derived-variables.sass and those derived variables are derived from inital-variables.sass.

So they exist, but are they imported?

src/styles/common-imports.scss brings in bulma-css-variables/sass/utilities/_all

But I can't see where common-imports.scss is imported or processed with sass, maybe @dpschen can shed some light on it, but that seems to be where all our missing variables are.

I would suggest not merging sticking plaster PR's like #1079 till this is resolved.

I would suggest not merging sticking plaster PR's like #1079 till this is resolved.

Yeah that sounds like a good idea. I think I've noticed other variables being undefined too, really looks like something is wrong here.

I have an idea what this issue is about.

But I can't see where common-imports.scss is imported or processed with sass, maybe @dpschen can shed some light on it, but that seems to be where all our missing variables are.

It's imported via the sass additionalData option in the vite.config.js

There are quite a lot of undefined custom properties.

@adrinux Which other variables then the mentioned are missing? Or more insterestingly: in which files are the missing variables defined?

I have an idea what this issue is about.

If I've understood it correctly it's to do with avoiding duplicating the css for every vue component?

It seems that the sass variables are correctly getting filled in as css variables during preprocessing but the bulma default values for those css variables don't get passed though?

I believe the chain is:

vite.config.js < src/styles/common-imports.scss < bulma-css-variables/sass/utilities/_all < bulma-css-variables/sass/utilities/derived-variables.sass < inital-variables.sass

@adrinux Which other variables then the mentioned are missing? Or more insterestingly: in which files are the missing variables defined?

--link, --text and other variable values are set in derived-variables.sass,
with the colour (and other variable) values they use defined in initial-variables.sass

So do the derived variable's values need to be imported into and set in the global.scss scope too?

Or if we're overriding half these colour variables to vikunja specific values anyway is it better to just put them all in custom-properties/colors.scss and be done?

If I've understood it correctly it's to do with avoiding duplicating the css for every vue component?

Yes

... but the bulma default values for those css variables don't get passed though?

I think they are. But only for the global.scss. Since the component styles are isolated they only have the SCSS variables available that get defined in common-imports.scss. CSS Custom properties should be available globally – if this is not the case maybe I miss something?

@adrinux Maybe you can try to fix it by increasing the specifiy of the custom properties so they always overwrite the ones defined by bulma-css-variables:

// in `custom-properties.scss`
html:root {} // Selector Specificity: (0, 1, 1)

// instead of

:root {} // Selector Specificity: (0, 1, 0)

So do the derived variable's values need to be imported into and set in the global.scss scope too?

Yes

Or if we're overriding half these colour variables to vikunja specific values anyway is it better to just put them all in custom-properties/colors.scss and be done?

Might also work.

Another way that might help fix this could be if we use bulvar and bulvar starts using the @use sass module syntax. I suggested this already.

I just realized that

html:root {}

doesn't select SVG elements. So custom properties would not be available in there.
I guess a better way to increase specifiy would be to repeat the selector:

:root:root {} // Selector Specificity: (0, 2, 0)

By the way this is also why it's usually recommended to use :root instead of html for custom properties that should be globally available. Wasn't aware of that =)

@adrinux Maybe you can try to fix it by increasing the specifiy of the custom properties so they always overwrite the ones defined by bulma-css-variables:

@dpschen This makes no sense to me. The error we see in browser dev tools is that '--link is undefined' and so on for other bulma variables too.

It's not a question of needing to override them - no value has been set for them!

And the reason we're seeing these problems such as with the date picker is that those elements were relying on bulma variables rather than anything ever being set for them in vikunja.

Yeah maybe I had a different imagination of that problem.

Nevertheless I still think that the @use "way" might solve these kind of problems.
And the bulvar lib now seems to have some progress in that regard: https://github.com/daniil4udo/bulvar/issues/38#issuecomment-981061391

I'm loosing track of all the places related to this issue. So I hope it makes sense to give an update here.

I wrote down the current status of my suspicion with the probs using bulvar here:
https://github.com/daniil4udo/bulvar/issues/38#issuecomment-989985535

Fixed in main, closing.