Theme: Revert three theme commits that are causing styling issues

[mgol]

This reverts three commits:

• 1b0e947 (Theme fixes #1753)
• dde9b83
• 265b8f5

which caused styling issues when compared to UI 1.12.1.

This unfixes a few issues:

• ThemeRoller: Clickable Default State and Icon do not generate any css download.jqueryui.com#335
• https://bugs.jqueryui.com/ticket/15059
• https://forum.jquery.com/topic/checkboxradio-widget-checkbox-click-doesn-t-work-with-ui-lightness-theme-22-9-2016

However, old & known issues are better than new & unknown ones, especially with our current very limited resources.

[fnagel]

Would be amazing if @jzaefferer could help a little here :-D

[fnagel]

I still feel a little sick when thinking about reverting fixes for bugs but I see that we definitely need to fix the unusable themes.

At least we should create a follow up ticket with all information we have. I can do this after my vacation. Maybe we'll find a solution sometime.

[fnagel]

Mhhh, I've tested this a little more and I see those issues: 1) a little glitch with the icon in radio / checkboxes (1.13 only) and 2) a fully broken checkbox in ui lightness theme (1.12 only) and 3) an ugly design glitch with radios in ui lightness theme (1.12 only)

Don't we get the broken checkbox in UI Lightness like in 1.12 when applying this PR?

[mgol]

@fnagel

At least we should create a follow up ticket with all information we have. I can do this after my vacation. Maybe we'll find a solution sometime.

Cool, I'll leave that for you to create then.

I did spend some time trying to fix the issues without reverting the changes but those commit are pretty substantial; they drastically change the ideas around which colors to use when and I wasn't able to see a quick way to resolve the issues. It might be that way bigger changes would be necessary, maybe even including changes in theme colors.

It's possible there's an easier & simpler way to fix the affected issues; that would be preferable.

Mhhh, I've tested this a little more and I see those issues: 1) a little glitch with the icon in radio / checkboxes (1.13 only)

Is that the visible dash that should be hidden? I'll prepare a separate PR for that one.

2. a fully broken checkbox in ui lightness theme (1.12 only)
3. an ugly design glitch with radios in ui lightness theme (1.12 only)

What's the glitch with radios?

Don't we get the broken checkbox in UI Lightness like in 1.12 when applying this PR?

We do. The last sentence of the PR description captures my thoughts around it:

However, old & known issues are better than new & unknown ones, especially with our current very limited resources.

[fnagel]

I did spend some time trying to fix the issues without reverting the changes but those commit are pretty substantial; they drastically change the ideas around which colors to use when and I wasn't able to see a quick way to resolve the issues. It might be that way bigger changes would be necessary, maybe even including changes in theme colors.

Mhh ok. The issue looks so small and fixable but I had no time digging into it yet. And I remember you already said something like its a tough one. Sorry for the duplicate question ^^

Is that the visible dash that should be hidden?

Yes

What's the glitch with radios?

In 12.1 it looks like this (the unchecked are more visible then the active one):

and like this in 1.13:

Anyway, this one is not a big problem IMO.

We do. The last sentence of the PR description captures my thoughts around it:
However, old & known issues are better than new & unknown ones, especially with our current very limited resources.

Hehe, yes, that sounds valid. I guess we don't have much options here :-/

[fnagel]

Maybe it's an option to add a workaround (e.g. some exrta CSS) to the ui-lighness theme to fix the 1.12 checkbox issue?

[mgol]

Maybe it's an option to add a workaround (e.g. some exrta CSS) to the ui-lighness theme to fix the 1.12 checkbox issue?

Themes are produced from https://github.com/jquery/jquery-ui/blob/main/themes/base/theme.css, mostly by changing colors to variables declared in comments like /*{borderColorContent}*/; those variables are defined in https://github.com/jquery/download.jqueryui.com/blob/main/lib/themeroller-themegallery.js for each theme. I don't see a way to add a special rule for a single theme, it's all driven by those theme variables.

All of the themes should be possible to generate using the Themeroller just by entering proper variable values; any special rule would violate such a constraint.

[mgol]

I submitted a fix for visible dashes in blank icons in #1987.

[mgol]

@fnagel the main issue is that two of the three mentioned commits change icon background styles to use border set to borderColorActive and background-color to bgColorContent - but in some themes, including the base one, those colors are very close to each other. This can only be fixed by changing the color definitions - which I want to avoid as that's pretty involved and I'm not a graphic designer - or by using different colors and then the question is which ones and that needs to be tested in all the 25 themes.

The second issue is the new .ui-state-default styles that override some icon styles, making them light grey instead of dark grey in the demo, it also causes many other weird differences.

[fnagel]

I don't see a way to add a special rule for a single theme, it's all driven by those theme variables.
All of the themes should be possible to generate using the Themeroller just by entering proper variable values; any special rule would violate such a constraint.

I already assumed it's not really possible to do this easily and you're right: it would not be a clean solution. More like a hack :-/

[fnagel]

I submitted a fix for visible dashes in blank icons in #1987.

This looks pretty good to me. Any easy way to test this locally with another theme?

[mgol]

@fnagel

This looks pretty good to me. Any easy way to test this locally with another theme?

There's a way, albeit a bit hacky.

First, you need to make sure the commit you want to test is on the origin, not on your fork. You may need to temporarily push a branch to the main jQuery UI repo. Then, copy the full commit ID of a commit you want to test.

Next, make sure you have the latest version of https://github.com/jquery/download.jqueryui.com. Then you need to open config.json and change the version field from 1.13.0-rc.2 to the commit ID you copied before. Delete the jquery-ui folder in the repo. I found that having both a recent commit ID in one entry and 1.13.0-rc.2 in a separate one doesn't work; the RC.2 version is served for the new entry as well. You can download the RC.2 themes from the official site for comparison since you won't have them in your local Download Builder due to this quirk.

Afterwards, run the following commands:

npm install
grunt prepare
node server.js --console

and open http://localhost:8080 in your browser. Click on the Download Builder link and you'll have a local version of the Download Builder. Choose 1.13.0-pre, pick a theme you want to check & click download. The downloaded package will have an index.html file with demos of all widgets.

[fnagel]

Uhhh ok, I hoped to test this before my vacation, which starts tomorrow morning, but I guess this one would take a little longer :-D

I might be able to do some testing in two weeks but no need to wait for me here. I assume I will just come to the same conclusion you did before.