Skip to content

Conversation

@susnux
Copy link
Contributor

@susnux susnux commented Feb 1, 2023

We can not set variables referencing themeable variables on the :root as currently the theme variables are applied on the body element. Meaning var() will be evaluated with the context of the :root element, which does not apply the user selected theme.
We neither can set this variables on the select, because the some of the variables are used to style the dropdown list which is a modal so not a child of the select.

This fixes issues like this:
nextcloud/forms#1471 (review)

See also nextcloud/server#36462

@susnux susnux added bug Something isn't working 3. to review Waiting for reviews design Design, UX, interface and interaction design feature: select Related to the NcSelect* components labels Feb 1, 2023
@susnux susnux force-pushed the fix/ncselect-theming branch from 1492527 to fec54a2 Compare February 5, 2023 14:07
@susnux susnux added this to the 7.6.0 milestone Feb 6, 2023
@susnux susnux requested a review from artonge February 6, 2023 15:24
Copy link
Contributor

@JuliaKirschenheuter JuliaKirschenheuter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would confirm, but @skjnldsv are you agree too?

@Pytal Pytal added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Feb 9, 2023
@Pytal Pytal merged commit d61fbf9 into master Feb 9, 2023
@Pytal Pytal deleted the fix/ncselect-theming branch February 9, 2023 18:18
@Pytal
Copy link
Contributor

Pytal commented Feb 9, 2023

I would confirm, but @skjnldsv are you agree too?

Probably AFK ⌨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish bug Something isn't working design Design, UX, interface and interaction design feature: select Related to the NcSelect* components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants