Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.First of all, the new option to select the starting menu item seems to work fine! :)
However, some weird styling was introduced somewhere, which completely breaks the form layout, rendering it almost unusable.
As the goal of this module is Drupal core, this styling should be removed IMHO, because it is not in line with any other form in Drupal core. If we can save some KB by doing this, even better.
I'll work on a patch tomorrow.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | menu_block.layout.patch | 4.87 KB | sun |
| #2 | menu-block-form.png | 13.35 KB | sun |
| menu_block-styling.png | 14.98 KB | sun | |
Comments
Comment #1
JohnAlbinWhich browser is this? I could have sworn I tested it on IE6, IE7, Firefox3 and Safari3.
Comment #2
sunFirefox 3/Windows. However, this issue is rather not related to a styling error, it's related to the styling at all. Counter question: Did you test any other theme than Garland? ;)
Really, I see no reason why Menu Block should suddenly start to introduce an own form layout/styling in this *part* of the block configuration form. If there was a completely separated form on an own page and the styling would actually be required to display the form elements properly, it would start to make sense. But this is not the case with Menu Block.
Admittedly, the screenshot was taken on a custom theme, which displays form elements in a two-column layout (label - element). However, all other forms of other modules are displayed correctly, so it's definitely not an issue with this theme.
...
After studying this styling in Firebug, it seems like the intention was exactly to introduce a two-column form layout. That's cool - but not really the job of this module. It's rather the job of a theme to apply appropriate styles - consistently - to all forms throughout Drupal.
Therefore, attached patch removes this styling. I'm also attaching another screenshot, which shows the default/resulting layout/styling of this form and hopefully convinces you that such styling is the job of the theme.
Comment #3
sunComment #4
sunBump!
John... please! This is not the job of a module, it's the job a theme... :)
Comment #5
sunRepeating myself once again:
It seems like the intention was to introduce a two-column form layout. That's cool - but not really the job of this module. It's rather the job of a theme to apply appropriate styles - consistently - to all forms throughout Drupal.
The current CSS only alters Menu Block's fieldset, but not the other fields in the same form, which makes the inconsistency obvious. Custom CSS would be appropriate if there was a special form containing form elements that require special placement to make any sense. However, the applied CSS styles are completely cosmetical and only serve to apply a different layout to this fieldset of the form. The implemented CSS presumes that forms are strictly using the styles applied by Drupal core (specifically Garland) only - which is a wrong assumption. By doing so, this breaks any other form layout that may have been applied globally to all forms in a Drupal site (overriding exactly those defaults by Drupal core to achieve a consistent, but different form layout).
I needed to apply this patch to 3 Drupal sites in the meantime. Not sure whether it still applies cleanly (it should), but I'm also happy to re-roll if required. At the very least, this is RTBC.
Comment #6
sunbump? :)
John, do you need co-maintainer for menu_block? :)
Comment #7
Nick_vhSun, this patch does not only adjust the styling but also the speed of the collapse.
could you clean up the patch so only the modified styles are included?
I think that than the possibility that the patch will be included is higher
Comment #8
JohnAlbinOk. I agree that CSS styling is too much on this form.
However, the reason I've been hesitant to commit it is because the form is really just too long and it becomes a Usability issue where the user can't figure out how the different elements relate to each other.
So, until I can completely re-do the form, I've committed a compromise. I've removed almost all of the CSS. And just added some CSS to add top and bottom margins to certain wrappers to allow breathing room around logical groups of elements. I've also removed some now-unnecessary form items.
Comment #9
sunyay! :)
btw, in general, relying on #edit-* ids is a not a good idea. Those ids are auto-generated and can change depending on external factors.
As far as the CSS is concerned, applying a .menu-block class to the form and possibly classes to the individual form elements would be better and reliable. (I'm specifically leaving out ids here, because it's harder to override those in a theme ;)
The same however also applies to the JS involved. (#states in D7 will most likely eliminate that entirely)
Comment #10
JohnAlbinOk. I refactored the jQuery to not use any IDs.
As for the CSS, if it breaks because the IDs change its no great loss since the styling is so minimal. And I don't see how the CSS could mis-apply since the CSS is only loaded on pages that include the menu block configuration form. Also, I don't see an obvious way to add classes to the div.form-item wrapper; #attributes adds classes directly to the element, not to its wrapper div.