In #2782885: No indication what page element is being configured with Outside In we introduced:
instance.options.data.dialogOptions = {outsideInActiveEditableId: $(instance.element).parents('.outside-in-editable').attr('id')};
This effectively wipes out any options that were added in link via:
$link['attributes'][''data-dialog-options']
One common option that is passed is width. So this would stop any other module that wanted to use the off-canvas tray at a different width.
This would be bring problem for #2784443: Move off-canvas functionality from Settings tray module into drupal.dialog.ajax library so that other modules can use it

Here is patch that fixes that and also updates OffCanvasTest to test that the width is respected.

CommentFileSizeAuthor
#9 interdiff-2804639-5-9.txt815 bytestedbow
#9 2804639-9.patch2.88 KBtedbow
#5 interdiff-2804639-3-5.txt658 bytestedbow
#5 2804639-5.patch2.88 KBtedbow
#3 offcanvas_width.patch2.81 KBtedbow
offcanvas_width.patch2.81 KBtedbow
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
Priority: Normal » Major

Bumping to major because it will stop #2784495: Normalize block place and outside-in experiences

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
FileSize
offcanvas_width.patch2.81 KB
GrandmaGlassesRopeMan’s picture

GrandmaGlassesRopeMan
he/him
Primary language English
Location 🌎
CreditAttribution: GrandmaGlassesRopeMan at Acquia commented 
+++ b/core/modules/outside_in/js/outside_in.js
@@ -143,7 +143,10 @@
+          if (!('dialogOptions' in instance.options.data)) {

Can we add some documentation around what's happening here.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
FileSize
2804639-5.patch2.88 KB
interdiff-2804639-3-5.txt658 bytes

Added comment re #4

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
Parent issue: » #2784443: Move off-canvas functionality from Settings tray module into drupal.dialog.ajax library so that other modules can use it
GrandmaGlassesRopeMan’s picture

GrandmaGlassesRopeMan
he/him
Primary language English
Location 🌎
CreditAttribution: GrandmaGlassesRopeMan at Acquia commented
Status: Needs review » Reviewed & tested by the community

@tedbow

This looks good to me.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Needs review 
+++ b/core/modules/outside_in/tests/src/FunctionalJavascript/OffCanvasTest.php
@@ -52,10 +52,13 @@ public function testOffCanvasLinks() {
+          self::assertTrue(stristr($style, 'width: 555px;') !== FALSE, 'Dialog width respected.');

Minor but why stristr() - can we not determine the case? Could use a comment possibly.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
FileSize
2804639-9.patch2.88 KB
interdiff-2804639-5-9.txt815 bytes

@catch no reason just mistake on my part. thanks for review.

Fixed.

GrandmaGlassesRopeMan’s picture

GrandmaGlassesRopeMan
he/him
Primary language English
Location 🌎
CreditAttribution: GrandmaGlassesRopeMan at Acquia commented
Status: Needs review » Reviewed & tested by the community
alexpott’s picture

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

Committed and pushed 552513f to 8.3.x and aa939b3 to 8.2.x. Thanks!

  • alexpott committed 552513f on 8.3.x 
    Issue #2804639 by tedbow, drpal, catch: Offcanvas dialog width not...

  • alexpott committed aa939b3 on 8.2.x 
    Issue #2804639 by tedbow, drpal, catch: Offcanvas dialog width not...

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! :)