Usability bug: down arrow of fieldsets in D7 overlays should be part of the link...

I personally have never seen a user do this in user testing, they always click the Legend anchor. Adding padding will not automatically include the arrow in the link - it has to be moved from the span wrapper to the actual link, which is a Drupal core issue and not one I would be hugely in favor of at this stage.

... which is a Drupal core issue and not one I would be hugely in favor of at this stage

Gotta agree with Jeff on this one. I'm a mac user and I've never thought to click that arrow on website ...

I'm windows and mac user, although these days I spend much more time on windows to be frank, maybe someone who is pure mac user might do this, but honestly its not something I have seen. I was a mac user for some 15 years or more before starting with windows just a few years ago, only sporadically did I use windows before that. FWIW and a bit of history my very first mac was an LCIII "pizza box" circa 1993, which replaced my Amiga 500... lol... ah the bad old days...

duplicate of
http://drupal.org/comment/reply/1305574

PS:
it DID matter to the DO designers, see the collapsible fieldsets on this page here...

I closed #1305574: Make arrows on collapsible fieldsets clickable too and am re-opening this one.

Instead of fiddling with the margins so the link overlaps the image, could the image itself be made a link?

Let's fix this.

#7, yes, its very simple, just move the image background from the span to the anchor, my admin theme does this, its a two second fix:

This is what my theme does (I have to override core)

html.js fieldset span.fieldset-legend {
  background-image: none !important;
  padding-left: 0 !important;
}
html.js fieldset.collapsible .fieldset-legend a {
  background: url('images/menu-expanded.png') no-repeat scroll 5px 65% transparent;
  padding-left: 15px;
}
html.js fieldset.collapsed .fieldset-legend a {
  background-image: url('images/menu-collapsed.png');
  background-position: 5px 50%;
}

just adding an "a" tag to the corresponding css instructions in system.theme.css seems to do the trick:

.js fieldset.collapsible > legend .fieldset-legend a{
.js fieldset.collapsed > legend .fieldset-legend a{

Jeff's solution is basically the same, only he used a slightly different selector.

PS:
I hope i got my first patch right.
Just "learned" how to contribute yesterday at the Swiss DUG Meeting in Zurich, where i also discussed this issue here with Lukas von Blarer.

@sirtet: Welcome! Glad your getting into the patching game :)

I dont hink patch names can have spaces in them. That might be why testbot seems to be unhappy. Give it another go without spaces and dont forget to change the issue back to "needs review" so testbot will know to come back and have another look at your new patch

Patch renamed

This needs before/after shot.

original:

patched:

RTBC!

Committed to 8.x. Thanks!

Committed to 8.x. Thanks!

Would it be OK to backport this?

D7 version of the patch from #13.

Setting to 7.x

#21: drupal-n948516-21-d7.patch queued for re-testing.

#facepalm. Thanks Jer.

as the code change is identical to the D8 patch, i guess i can set this to RTBC?
(I did successfully download and apply the patch from #21)

I think we can make an exception to backport this since it's such a minor markup adjustment. If David disagrees, he should feel free to revert this patch.

Committed and pushed to 7.x. Thanks!

newbie question: what's the "exception" here?
I guess it's something like that this is not seen as a bug, but just a style question, where styles are normally not changed/backported?

sirtet: we typically try to avoid changing the js behaviors (and markup, and translatable strings, and API functions, and a couple other things...) on a released version of Drupal. In this case webchick decided that the change was minor enough (and unlikely to be something that could break any existing sites out there) so she committed it.

Had this change been to remove the arrow completely or renamed the classes used by it I doubt she would have committed it...

I just rolled this back from Drupal 7 not for the reasons discussed above, but rather because it looked to me like the patch forgot to deal with right-to-left languages.

I wasn't sure how bad that omission would be, so I tested it and (see the attached screenshots) the answer is "very bad" :) Hopefully this isn't too hard to fix, but it needs to go in Drupal 8 first.

Also, before backporting it to Drupal 7 again, we probably should discuss the backport a little more? The thing about fieldsets is that they are very commonly used not just on admin screens but also on public-facing pages of a website, and thus they sometimes can be heavily themed. So I think we should be especially careful backporting changes unless they're really necessary. Just from looking at the patch, it's hard for me to get a sense of how likely it is to break a theme which has heavily customized its fieldsets, but maybe someone else has a better idea?

Oh, also, another backport note: The patch here looks like it conflicts with #1361810: Non-collapsible fieldsets within collapsed fieldsets have "expanded" icon which might get committed to Drupal 7 soon.

Hopefully the Drupal 8 fix is as simple as this.

It's untested for Drupal 8, but the same change does seem to work correctly in Drupal 7 (figured I'd test it there quickly while I already had the RTL language set up in my development environment).

The only reason I'm not sure is that the CSS rules for LTR and RTL languages look a fair amount different in this part of the code, so perhaps a more complicated change is actually required.

Wow, thanks for catching that, David! So sorry. :(

yes, the patch from #32 does work in D8:

Looks like that's a review! :)

This does not seem to be an issue in latest Drupal 8 beta ---- closing as cannot reproduce

Looks like the regression is no longer present (probably was fixed when Drupal 8 changed the way it handled RTL styling to no longer use separate CSS files) but we still need to consider the original patch (plus fixes to prevent the regression) for possible Drupal 7 backport.

I know this is a really old D7 ticket, but for record's sake: I had no trouble with RTL output when I applied the changes in #10 to the rules in both system.theme.css AND system.theme-rtl.css. Could my reformed patch get put into the next release of D7?