Problem/Motivation

On Create Basic Page, try clicking the container around the Menu Settings menu item. Although it has a pointer, it doesn't click like the link does. (See attached image).

<details id="edit-menu" class="menu-link-form form-wrapper collapse-processed">
  <summary aria-controls="edit-menu" role="button">
    <a class="details-title" href="#edit-menu"><span class="details-summary-prefix visually-hidden">Show</span> Menu settings</a>
    <span class="summary"> (Not in menu)</span>
  </summary>
  ..
</details>

The a tag is clickable, and expands the div class="details-wrapper" below, but the summary does not.

Proposed resolution

Make the summary the clickable element, instead of the a tag inside of it

CommentFileSizeAuthor
#13 fix_summary_click_event-2467559-13.patch544 bytespuddyglum
#10 fix_summary_click_event-2467559-10.patch545 bytespuddyglum
#6 fix_summary_click_event-2467559-6.patch552 bytespuddyglum
#5 fix_summary_click_event-2467559-5.patch611 bytespuddyglum
#3 d8-summary-click.png16.21 KBpuddyglum
#2 fix_summary_click_event-2467559-2.patch579 bytespuddyglum

Comments

puddyglum’s picture

puddyglum
Primary language English
Location Elk Grove, CA
commented
Issue summary: View changes
puddyglum’s picture

puddyglum
Primary language English
Location Elk Grove, CA
commented
Component: forms system » javascript
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new fix_summary_click_event-2467559-2.patch579 bytes

Changed the click event from the link to the summary wrapper.

puddyglum’s picture

puddyglum
Primary language English
Location Elk Grove, CA
commented
Title: "Summary" wrappers are not clickable on node form, edit entity meta panels » "Summary" wrappers around links are not clickable on node form's edit entity meta panels
Issue summary: View changes
Issue tags: +Needs accessibility review
StatusFileSize
new d8-summary-click.png16.21 KB

Added a screenshot

nod_’s picture

nod_
Primary language French
Location Lille
commented
Issue tags: -Needs accessibility review

That works.

Minor nitpick, can you format the code similarly to what's around? 

      $legend
        .append(this.$summary)
        .on('click', $.proxy(this.onLegendClick, this));

After that change It's RTBC for me. There is no change on accessibility, the link can be focused and works the same as before (event bubble up to the summary, which triggers the behavior).

puddyglum’s picture

puddyglum
Primary language English
Location Elk Grove, CA
commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new fix_summary_click_event-2467559-5.patch611 bytes

Better formatting per nod_

puddyglum’s picture

puddyglum
Primary language English
Location Elk Grove, CA
commented
StatusFileSize
new fix_summary_click_event-2467559-6.patch552 bytes

Removed extra ;

nod_’s picture

nod_
Primary language French
Location Lille
commented

It's a bit frowned upon to RTBC your own patches. It's smallish and I did say it was RTBC, just keep it in mind when working on larger patches :)

RTBC comfired.

puddyglum’s picture

puddyglum
Primary language English
Location Elk Grove, CA
commented

Doh! Thanks for the heads up

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/misc/collapse.js
@@ -59,9 +59,11 @@
+        .appendTo($legend);        ¶

There seems to be a lot of whitespace or a tab added to the end of line here.

puddyglum’s picture

puddyglum
Primary language English
Location Elk Grove, CA
commented
StatusFileSize
new fix_summary_click_event-2467559-10.patch545 bytes

Whitespace removed

puddyglum’s picture

puddyglum
Primary language English
Location Elk Grove, CA
commented
Status: Needs work » Needs review
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs review » Needs work 
+++ b/core/misc/collapse.js
@@ -59,9 +59,11 @@
+        .appendTo($legend); ¶

There still is a single whitespace character. You can run eslint to test js coding standards.

eslint ./

core/misc/collapse.js
  62:28  error  Trailing spaces not allowed  no-trailing-spaces

✖ 1 problem (1 error, 0 warnings)
puddyglum’s picture

puddyglum
Primary language English
Location Elk Grove, CA
commented
StatusFileSize
new fix_summary_click_event-2467559-13.patch544 bytes

Thanks, removed the extra space and tested with eslint

puddyglum’s picture

puddyglum
Primary language English
Location Elk Grove, CA
commented
Status: Needs work » Needs review
nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs review » Reviewed & tested by the community
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed c889a5f and pushed to 8.0.x. Thanks!

  • alexpott committed c889a5f on 8.0.x 
    Issue #2467559 by jmonkfish: "Summary" wrappers around links are not...

Status: Fixed » Closed (fixed)

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