themeroller.jquerymobile.com icon indicating copy to clipboard operation
themeroller.jquerymobile.com copied to clipboard

import does not retain and display theme where background and border of buttons matches

Open alexjeffburke opened this issue 10 years ago • 8 comments

Hi there,

I exported a theme where the background colour and border colour of buttons was set the same. On import, the button border is reset. I verified it exported correctly by previewing the generated index.html file from the theme archive which correctly displays the border.

Thanks, Alex J Burke.

alexjeffburke avatar Feb 17 '15 15:02 alexjeffburke

@alexjeffburke Thanks for reporting the issue. I can confirm the problem.

@gabrielschulhof This is a regression from https://github.com/jquery/themeroller.jquerymobile.com/commit/8c592fa1c8f4abd039559978c9f9ba300e19ab99. I tested local without that code and the problem was gone. TR.upgrading && TR.versionCompare( TR.upgrading, "[1.4-)" ) shouldn't be true when importing a theme while the TR is set to version 1.4.x.

jaspermdegroot avatar Feb 19 '15 21:02 jaspermdegroot

Hi - I believe this issue is still in the wild, is there anything I can do to help this along?

alexjeffburke avatar Mar 08 '15 21:03 alexjeffburke

@alexjeffburke PR's are always welcome.

arschmitz avatar Mar 08 '15 21:03 arschmitz

Ok.

Could you provide me some guidance here? The commit identified earlier in this thread seems to purposefully reset the borders. Personally I'd remove the offending chunk, if someone has explicitly requested the background and borders match that information should not be lost on import.

However, I'm also lacking context in that I don't know why this change was added. Is it possible this is intended to be for themes from jQM < 1.4 where the border was always the same colour? If so, perhaps the right fix is if you are already on a 1.4 version and you've made your choices this block of code need not be run.

Thanks, Alex J Burke.

alexjeffburke avatar Mar 13 '15 10:03 alexjeffburke

Hello,

Think the behavior is still there. My Posting from the jQuery-mobile Bugtracker was closed to leave a comment here:

We found a wrong behaviour of the jQuery Mobile Themeroller Export. If we export a created theme with changed border-colors we have to change the colors manualy after exporting it.

5db4d108-2fc6-11e5-8590-f00125f982f1

In the Screenshot you see the difference between the original code on the right side and the exported code on the left side, without making a change in the UI. This difference are in every swatch we made so that it is not easy for us to use an exported theme directly for our customers.

Hope you can work with this and fix this bug in one of the next versions of the Themeroller.

bocksebastian avatar Jul 24 '15 07:07 bocksebastian

Yes, it's definitely still a problem. The commit that introduced the behaviour was identified, but I received no response when I asked how to go about submitting a PR.

It certainly seems like theme roller isn't really supported any more - for the record, the import process is largely written in JS so the last time I had to use it I jumped into the chrome debugger and live patched the code so it never changes the border colour before importing the theme. This worked well for me.

alexjeffburke avatar Jul 24 '15 08:07 alexjeffburke

We are working with GIT here to. So perhaps you can show me the the code to correct an i will do the PR for you.

bocksebastian avatar Jul 29 '15 06:07 bocksebastian

Thanks for the offer - the reason I didn't submit a PR was I have no idea what a 'correct' fix would be. Again, I'm pretty sure 8c592fa1c8f4abd039559978c9f9ba300e19ab99 is responsible for it.

My guess (without knowing all the details) is at some point the norm for themes changed from using the same colour to separate ones. So the upgrade code was changed to identify this and apply the new standards ..except it is applied in cases where someone might have explicitly chosen to save a 1.4 theme where button borders match their background colours. This means the update process incorrectly alters such a theme. As @jaspermdegroot said, perhaps the version check should be less than 1.4 or if the values were set explicitly not get applied?

alexjeffburke avatar Sep 05 '15 17:09 alexjeffburke