Bartik, do not import style.css from preview.css

We need some more testing here, I can definitely see the potential for breakages - also the style.css changes the look of the admin theme on this one page which doesn't seem right to change/modify the look of the admin theme.

While the changes are not great at the moment, this could be bad if the admin theme is changed and there are major conflicts, or if the user copies Bartik and makes changes - which could lead to unpredictable results in the admin theme.

Only real solution I can see is to rip the preview stuff out of style.css and style the preview entirely from preview.css (and not load style.css in the iframe at all) - while this means code duplication and more overhead its probably the only safe way around this. It does leave our style.css file a lot cleaner so that's one advantage.

We have some immediate issues:

1) we need a font-size insulator - the only really safe way is to use a keyword and set everything relative from that - this is quite laborious because it needs some work to "emulate" Bartiks font sizes for the preview.

2) a markyup cleanup - right now the themes markup does not match the previews markup - we made changes to the footer columns markup recently so we'll need to emulate that also.

3) probably stuff I've missed so this needs work.

Setting to major - but this could easily be critical, while its not smashing up Seven right now, it could.

The patch is a start - it needs a lot of work and testing - please help me, I'm overrun with issues...

sending for review, could need a re-roll.

Reroll.

For the record, I have not reviewed this yet, I just rerolled it.

#5: drupal-bartik-844734-5.patch queued for re-testing.

I think it would be pretty cool if we showed the main menu tabs in the preview.

OK we need to bump this along, I have found one situation where this destroys Legend styles - IE7. If you load the setting page for Bartik the fieldset legends are wreaked (with Seven as admin theme).

I would really really like the Borked Header patch to go first though, then this one - both are major but the other one is higher on my hit list.

This, being major (page destroying major) probably needs to go next, I'm gonna re-roll this and see what I can come up with. I'm a pretty grumpy the moment because Bartik is destroying my lovely admin theme, grrrrrrrr...

Just to highlight the issue at hand, take a look at what Bartik does to my otherwise beautiful Admin theme...

OK, so I worked on this tonight for hours and hours and came to the conclusion that this whole idea of an HTML preview is a massive PITA, but we're stuck with it so we got to make it work, somehow. Honestly though I think at some stage things are going to break, its just so hard to insulate against every possible inheritance issue.

This patch does two 4 major things:

1. Removes the @import from preview CSS, which as importing style.css (very bad move, see the screenshot in 11 for an example)

2. Moves all the preview styles from style.css to preview.css, agruably where they belong and should have been all along.

3. Fixes many issues with the preview.html file, there were coding errors (broken HTML) and the markup did not match that of Bartik anyway.

4. Removes Bartiks custom styles for the color form, these badly broke the form in when displayed in my admin theme and are just too fragile. I added some styles to adjust the width of the form item and form label so bartiks long form labels fit on one line. Essentially this restores the form to core styles which are very robust.

One big change is that in preview.html I abandoned the idea of trying to copy the live themes HTML and instead trimmed it right back to only what we need in the actual preview. This means we have much less markup to worry about and its far far easier to debug it and somewhat mitigates the risk of a stray inheritance issue.

And just for added class, I added the tabs to the preview, which really makes it shine - see the screenshot (too big to embed).

Just spotted a few improvement to mitigate possible inheritance issues so re-rolled. Adding a screenshot to show that all the colorable bits still work in the preview (I had to make adjustments to preview.js for this all to work correctly).

#13: bartik-preview-css-html-844734_3.patch queued for re-testing.

#13: bartik-preview-css-html-844734_3.patch queued for re-testing.

Jeff, can you give us some guidance on how to review this patch? What should we be looking for/ testing once it's applied?

Basically need to look for a couple of things:

1 - Preview is working (there are JS changes in this patch)
2 - Color form is not broken, especially when resizing text up to 200%
3 - the Admin theme, whatever that is, is not affected - right now its easy to see how Seven is modified by the imported stylesheet.

The preview is the main thing, cross browser testing is probably needed although I know I did some. The preview had by far the most work, the HTML is total rewrite as is most of the CSS and some JS. This needed to happen because the HTML was miss-matched to page.tpl.php to begin with, had some broken HTML and was overly verbose for the task at hand. Its very much simplified in this patch and should be easier to maintain - in fact it won't need changing at all, even if we switched to HTML5 this would still be valid, only a design change would dictate a change here.

I fully tested this, but I had to re-roll, so it needs someone else to RTBC.

Re-rolled for minor whitespace changes.

Patch attached.

This is mostly catching up the preview with the changes made to Bartik. It's important because it makes the color.module worth using.

Its a little more than catching up with changes made to Bartik - its a complete change to how previews are built - the HTML is totally changed, the CSS moved - which changes the concept behind how the previews are styled - previously we had this rather misguided idea of importing style.css into preview.css and re-using the themes CSS, whereas this approach abandons that as un-workable (which it is) and instead places preview styles entirely in preview.css.

This fixes many issues created by importing style.css (many styles clash with the admin themes CSS), simplifies the HTML markup for the preview and the preview.css.

So my summing up wasn't 100%. But we definitely got preview to more accurately reflect the way Bartik can look. Also, this was committed already :)

http://drupal.org/cvs?commit=451034