What was supposed to be a feature is really a bug. I'm not sure what the background is to deciding to add a class of "element-invisible" to the Body field label, but this makes it impossible to use the summary toggle.

Why was this done?

CommentFileSizeAuthor
#7 panopoly_admin-hide-body-label-better-2461077-7.patch2.54 KBdsnopek
#6 2461077-panopoly_admin-body-summary-toggle-6.patch2.54 KBdshields
#5 2461077-panopoly_admin-body-summary-toggle-5.patch2.55 KBdshields

Comments

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Related issues: +#2396241: Hide field labels in a more accessible way

We've always hidden the body label. We used to do it using display: none in CSS Javascript, which wasn't accessible. So, we switched it to using the 'element-invisible' class which is accessible (ie. it'll get read by screenreaders). Here's the original issue:

#2396241: Hide field labels in a more accessible way

Unfortunately, no one noticed that it had collateral damage on the summary toggle. :-/ I'm not sure what the fix is yet...

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented

It turns out we used to do this in Javascript and actually forgot to remove the Javascript code. From #2396241-13: Hide field labels in a more accessible way:

Hrm, I just realized that this didn't remove the Javascript in panopoly-admin.js which clears out the Body label! So, the Body label is still not accessible. We can fix this over in #2461077: Hiding the Body field label breaks the summary toggle because that's already a follow-up to this issue.

dshields’s picture

dshields commented

A good solution would be to not remove the body label.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented

It probably should be made optional, but this is a feature we need to continue to support (and it shouldn't break the summary toggle when used!)

dshields’s picture

dshields commented
Status: Active » Needs review
StatusFileSize
new 2461077-panopoly_admin-body-summary-toggle-5.patch2.55 KB

Skip this patch

dshields’s picture

dshields commented
StatusFileSize
new 2461077-panopoly_admin-body-summary-toggle-6.patch2.54 KB

Oops - try this patch instead. In the first one, I removed a line that I had previously commented out. It should just be removed.

If you need to support that feature, this patch may get you started on a fix that works. I'm not quite sure why the feature needs to be supported, but..

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
StatusFileSize
new panopoly_admin-hide-body-label-better-2461077-7.patch2.54 KB

Thanks for the patch!

Here's a new patch inspired by yours, except with the following changes:

  • It applies against the latest Panopoly -dev (yours was against Panopoly 1.19)
  • If the Body field doesn't allow summary input, it hides the label the simple way in the Form API
  • It doesn't try to preserve the required marker to match the behavior in Panopoly 1.18 and earlier

Anyway, best I can tell this patch will bring us back to the same behavior that existed in Panopoly 1.18 and earlier!

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Version: 7.x-1.19 » 7.x-1.x-dev
Status: Needs review » Fixed

Committed! Thanks again for your help on this. :-)

  • dsnopek committed b52cbb9 on 7.x-1.x 
    Update Panopoly Admin for Issue #2461077 by dshields, dsnopek: Hiding...
dshields’s picture

dshields commented

Happy to help :-)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.