follow up to #2821263: If a toolbar item besides the admin menu is open when going into edit mode it is not closed.

It works now but doesn't use the correct method therefore is more likely to break.

CommentFileSizeAuthor
#5 2843901-2.gif250.81 KBGrandmaGlassesRopeMan
#5 2843901-1.gif101.56 KBGrandmaGlassesRopeMan
#2 2843901-2.patch616 bytestedbow
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tedbow created an issue. See original summary.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
FileSize
2843901-2.patch616 bytes

This is @droplet's fix not mine. Thanks for it

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
Status: Active » Needs review
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented

Note for reviewer.

Since this patch is not introducing new functionality, just correcting Javascript to be more correct there is already test coverage in \Drupal\Tests\outside_in\FunctionalJavascript\OutsideInBlockFormTest::testBlocks to test that different open toolbar items are closed when entering edit mode.

The tests still pass.

To manually test it doesn't break that functionality

  1. Enable Setting Tray module
  2. Goto home page
  3. Click "Manage" to open the toolbar item(if not already open"
  4. Click "Edit" in the toolbar
  5. Confirm the "Manage" toolbar item is closed.
  6. Repeat above steps with Shortcut, and user toolbar items.
GrandmaGlassesRopeMan’s picture

GrandmaGlassesRopeMan
he/him
Primary language English
Location 🌎
CreditAttribution: GrandmaGlassesRopeMan at Acquia commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
FileSize
2843901-1.gif101.56 KB
2843901-2.gif250.81 KB

This looks good.

  • webchick committed b47dc9e on 8.3.x 
    Issue #2843901 by tedbow, drpal: Settings tray should use toolbar models...

  • webchick committed d02d6f9 on 8.2.x 
    Issue #2843901 by tedbow, drpal: Settings tray should use toolbar models...
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented
Status: Reviewed & tested by the community » Fixed

Committed b47dc9e and pushed to 8.3.x and cherry-picked to 8.2.x. Thanks!

droplet’s picture

droplet CreditAttribution: droplet commented

This is missing my credit. LOL

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented

Oops! Fixed. :)

  • webchick committed b47dc9e on 8.4.x 
    Issue #2843901 by tedbow, drpal: Settings tray should use toolbar models...

  • webchick committed b47dc9e on 8.4.x 
    Issue #2843901 by tedbow, drpal: Settings tray should use toolbar models...

Status: Fixed » Closed (fixed)

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

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
Component: outside_in.module » settings_tray.module

Changing to new settings_tray.module component. @drpal thanks for script help! :)