Text format selection layout is broken.

it looks like the javascript was broken when someone changed the <label> syntax to the <h3> syntax.

Alllright!

I've added another patch that tackles the styling issues as well.

works for me...

oops, too early. Bot isn't finished testing yet.

+++ modules/filter/filter.js	22 Aug 2010 05:04:57 -0000
@@ -7,7 +7,7 @@
+      .find('h3').hide()

badness all round, do not use .hide(), use .addClass() and .element-invisible. There is no point in even having a heading if no one can read or hear it.

Powered by Dreditor.

@Jeff Burnz, that would be a new feature, and not necessarily a desirable one. The patch by BarisW in #5 retains the previous behavior (before the bug was introduced. The point of the headings is in case JavaScript is disabled, one can still tell which filter tips are for which formats. In other cases, it is superfluous because the setting of the select element already tells you which format it's for. Not sure why the labels show up in the screenshots, they shouldn't be there unless JavaScript is disabled, and they don't show up for me with JavaScript enabled. Besides that, to me this is rtbc.

How stupid of me. The reason that the heading is not hiding had something to do with another issue I'm working on (changing the H3 to H2 on a node/add form), #890212: text format help text uses wrong headings.

I changed the jQuery selector from being heading-specific (H2) to being a heading-selector (:heading). Makes more sense. This patch hides the heading completely ;)

@cwgordon7, ar Ok, very good explanation. Thank-you. Indeed this looks good.

#10: 889764_input_filter_tips_2.patch queued for re-testing.

Allright, anyone wanting to test this?
It took some time, but now testbot returns green!

@kostask tested it (in the code sprint) and found a bug. Fixed and repatched!

Call me stupid, but the patch in #10 doesn't solve the initial issue, namely help-texts shown for all input formats all the time. Is this intended? The help-texts for input formats that are not selected are still there.

@reglogge: You're not stupid. I used the wrong jQuery selector $(':heading') instead of $(':header').
Would you be so kind to te-test with the new patch?

Cross-post. Now it works.

Sigh. I forgot to include the small change in seven/style.css. Added it back to v3.
Should be done now!

I tested the patch at #20 and everything seems to be working fine.

Looks good to me as well.

Thanks for testing!

Lets remove the tag, as it is cleared it #13.

+++ modules/filter/filter.css	23 Aug 2010 13:54:02 -0000
@@ -28,7 +28,16 @@
+#edit-body .filter-guidelines {
+  padding-top: 0.5em;
+}
+#edit-comment-body .filter-guidelines{
+  padding-left: 1.5em;
+}

Under no circumstances we will implement module or field specific styles into filter.css or elsewhere.

+++ modules/filter/filter.js	23 Aug 2010 13:54:02 -0000
@@ -7,7 +7,7 @@
-      .find('label').hide()
+      .find(':header').hide()

This seems to be the actual fix, so what are all the other changes for?

Powered by Dreditor.

The point is that style.css in Seven resets the padding of .filter-guidelines., which makes the guidelines touches the dropdown (ugly).
Is it better to change the padding in style.css?

See screenshots

If Seven resets the styles, then it also has to set them.

Sounds like something even I can handle. Attached patch should result in this:

+++ themes/seven/style.css	12 Sep 2010 09:16:23 -0000
@@ -609,12 +609,13 @@
 .filter-wrapper > div {
   padding-right: 6px;
   padding-left: 6px;
+  padding-bottom: 0;
 }

Why use the child selector here - we can easily support IE6 as well. The padding can be short handed.

.filter-wrapper .fieldset-wrapper {
  padding: 0 6px;
}

Otherwise looking good to go.

Powered by Dreditor.

No idea about the child selector, was already in there. Can indeed be shorthanded. I'm not working on this the next couple hours… :-)

Sure, I can whack this out easy enough :)

#32 patch works as expected in latest opera, Firefox, Chrome, IE8 in windows
Do we mark this as RTBC ?

Please check the guidelines display on the comment form as well. I recall I made some changes in my first patch for this case as well. I'm going on a vacation in an hour, so I don't have anymore time to do this.

Tips on the comment form would mean Seven being used as the front end theme. Unlikely scenario but looking fine: http://skitch.com/yoroy/dsjjc/sdfsdfsdf-d7

Rtbc indeed.

Checked with sun and he was on board. Committed to CVS HEAD. Thanks all.