We're using Layout Builder to allow editors to add custom blocks to a page. One of our custom blocks has a multi-value term reference field on it, with the form widget rendered as a combobox (multi-select). In Chrome, the list items "escape" the bounding box and overflow vertically.

Screenshot of what it should look like (Seen in Safari/Firefox/Edge):

correct

Here's what it actually looks like in Chrome:

incorrect

Steps to reproduce:

  1. Install fresh site from 8.6.x or 8.7.x and use "Standard" profile
  2. Enable Layout Builder
  3. Add at least 5 terms to the existing "Tags" vocabulary
  4. Add a custom block type and add a term reference field to the tags vocabulary. Allow unlimited items
  5. Update form display widget for that block type to use a select list
  6. Enable layout builder for the existing "Basic Page" content type default entity view display
  7. Edit layout, add block, and select "Add Custom Block"
  8. Add the new block you created, and observe the issue

This seems to be related to the "-webkit-appearance: menulist;" CSS property that's added in off-canvas.form.css. Once I removed that, it looks like it does in FireFox and Safari.

CommentFileSizeAuthor
#15 3043467-multi_select_list_items_escape_bounding_box_in_off_canvas_tray-15-interdiff.txt1014 bytesAaronMcHale
#15 3043467-multi_select_list_items_escape_bounding_box_in_off_canvas_tray-15.patch960 bytesAaronMcHale
#12 3043467-multi_select_list_items_escape_bounding_box_in_off_canvas_tray-12-interdiff.txt455 bytesAaronMcHale
#12 3043467-multi_select_list_items_escape_bounding_box_in_off_canvas_tray-12.patch982 bytesAaronMcHale
#9 multi-select-list-items-escape-bounding-box-in-off-canvas-tray-in-chrome-3043467-9.patch527 bytesmgbauman
Screen Shot 2019-03-26 at 10.17.30 AM.png6.13 KBbkosborne
Screen Shot 2019-03-26 at 10.17.40 AM.png6.31 KBbkosborne
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bkosborne created an issue. See original summary.

bkosborne’s picture

bkosborne
Primary language English
Location New Jersey, USA
CreditAttribution: bkosborne as a volunteer and at Princeton University commented
Issue summary: View changes
bkosborne’s picture

bkosborne
Primary language English
Location New Jersey, USA
CreditAttribution: bkosborne as a volunteer and at Princeton University commented
Issue summary: View changes
bkosborne’s picture

bkosborne
Primary language English
Location New Jersey, USA
CreditAttribution: bkosborne as a volunteer and at Princeton University commented
Component: settings_tray.module » CSS

Not sure, but I think this should be CSS component and not settings tray.

AaronMcHale’s picture

AaronMcHale
Primary language English
Location Edinburgh, Scotland
CreditAttribution: AaronMcHale at The University of Edinburgh commented
Issue summary: View changes

Adding Edge to the list along with Sarari and Firefox where this doesn't break, as confirmed this bug is not there in Edge.

AaronMcHale’s picture

AaronMcHale
Primary language English
Location Edinburgh, Scotland
CreditAttribution: AaronMcHale at The University of Edinburgh commented
Version: 8.6.x-dev » 8.8.x-dev

Bumping version to 8.8.x, might also be able to back-port to 8.7 if a patch passes before 8.8 release.

tdroden’s picture

tdroden CreditAttribution: tdroden at University of Calgary commented

We've recently run into this, and I believe I have a simple patch for it, but would need to be applied against web/core/misc/dialog/off-canvas.form.css &/or web/core/themes/stable/css/core/dialog/off-canvas.form.css. I'm not sure which, or is it both CSS files? Any insight would be greatly appreciated.

bkosborne’s picture

bkosborne
Primary language English
Location New Jersey, USA
CreditAttribution: bkosborne as a volunteer and at Princeton University commented

From what I understand, the stable theme is never supposed to change existing rules like that (to prevent surprises from themes that extend from it). But it would certainly need to be updated in the web/core/misc/dialog/off-canvas.form.css file.

mgbauman’s picture

mgbauman CreditAttribution: mgbauman commented
FileSize
multi-select-list-items-escape-bounding-box-in-off-canvas-tray-in-chrome-3043467-9.patch527 bytes

I think changing the selector to not include multiselects in web/core/themes/stable/css/core/dialog/off-canvas.form.css will work. That way, regular selects will still get -webkit-appearance: menulist; and -moz-appearance: menulist; but multiselects will not.

mgbauman’s picture

mgbauman CreditAttribution: mgbauman commented
Status: Active » Needs review

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

AaronMcHale’s picture

AaronMcHale
Primary language English
Location Edinburgh, Scotland
CreditAttribution: AaronMcHale at The University of Edinburgh commented
Version: 8.9.x-dev » 8.8.x-dev
FileSize
3043467-multi_select_list_items_escape_bounding_box_in_off_canvas_tray-12.patch982 bytes
3043467-multi_select_list_items_escape_bounding_box_in_off_canvas_tray-12-interdiff.txt455 bytes

Alright, let's see if we can get this tiny but useful fix into 8.8 at the very last second :D

Patch adds the fixed CSS to core/misc/dialog/off-canvas.form.css as well as core/themes/stable/css/core/dialog/off-canvas.form.css.

Also for the record, there was a brief discussion in Slack involving myself, benjifisher, dww, webchick and lauriii about this issue, and there seemed to be consensus that it should be fine to make this change in stable because it's a bug fix, it's a very small change, and it only impacts the off canvas dialogue / settings tray, so it's very unlikely that a theme would have customised this. It's also worth noting that #2659890: [Policy] [Plan] Drupal 9 and 10 markup and CSS backwards compatibility is trying to solve the dilemma of when and where it's ok to make CSS changes.

AaronMcHale’s picture

AaronMcHale
Primary language English
Location Edinburgh, Scotland
CreditAttribution: AaronMcHale at The University of Edinburgh commented

Started on a CR, but will have to finish it later today as need to for now. https://www.drupal.org/node/3096346

bkosborne’s picture

bkosborne
Primary language English
Location New Jersey, USA
CreditAttribution: bkosborne as a volunteer and at Princeton University commented

Are we sure that [multiple="multiple"] is needed and not just [multiple]? I've been using a version of this patch that just targets [multiple] and it's fine

AaronMcHale’s picture

AaronMcHale
Primary language English
Location Edinburgh, Scotland
CreditAttribution: AaronMcHale at The University of Edinburgh commented
FileSize
3043467-multi_select_list_items_escape_bounding_box_in_off_canvas_tray-15.patch960 bytes
3043467-multi_select_list_items_escape_bounding_box_in_off_canvas_tray-15-interdiff.txt1014 bytes

Well spotted @bkosborne, according to W3Schools the multiple attribute doesn't take any value: https://www.w3schools.com/tags/att_select_multiple.asp. Uploaded a new patch and interdiff.

AaronMcHale’s picture

AaronMcHale
Primary language English
Location Edinburgh, Scotland
CreditAttribution: AaronMcHale at The University of Edinburgh commented

Added a description of the change (along with the screenshots in the IS) to the draft change record at https://www.drupal.org/node/3096346

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented

Should we open a follow-up to come up with a better design for the multi-select list? This issue at least makes the element usable on Chrome which is a big step forward! 👏

AaronMcHale’s picture

AaronMcHale
Primary language English
Location Edinburgh, Scotland
CreditAttribution: AaronMcHale at The University of Edinburgh commented
Related issues: +#3096861: Improve the design/UX of multi-value select elements in Off Canvas Dialogue forms

Should we open a follow-up to come up with a better design for the multi-select list?

Yes that sounds like a good idea, for example the big white border as it is now isn't really consistent with the off canvas / settings tray look, opened #3096861: Improve the design/UX of multi-value select elements in Off Canvas Dialogue forms as a follow-up.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Needs review » Reviewed & tested by the community

Awesome! Let's get this done first.

I think this change to Stable is fine. It does increase the specificity of the selector but the selector is only using -webkit-appearance and -moz-appearance properties so it seems something that would have just a small risk of colliding with something else.

AaronMcHale’s picture

AaronMcHale
Primary language English
Location Edinburgh, Scotland
CreditAttribution: AaronMcHale at The University of Edinburgh commented
Title: Multi-select list items "escape" bounding box in off-canvas tray in Chrome » Multi-select list items "escape" bounding box in Off-Canvas Forms when using WebKit and Mozilla based browsers

Better title.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Patch (to be ported)

Committed and pushed c9af9e0fdb to 9.0.x and 2d5621de71 to 8.9.x. Thanks!

Gonna ping @lauriii about backporting this.

  • alexpott committed c9af9e0 on 9.0.x 
    Issue #3043467 by AaronMcHale, mgbauman, bkosborne, lauriii: Multi-...

  • alexpott committed 2d5621d on 8.9.x 
    Issue #3043467 by AaronMcHale, mgbauman, bkosborne, lauriii: Multi-...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Patch (to be ported) » Fixed

@lauriii and I agreed to backport this to 8.8.x. I can't really imagine what this is going to break in a worse than HEAD is already broken.

  • alexpott committed 20cc893 on 8.8.x 
    Issue #3043467 by AaronMcHale, mgbauman, bkosborne, lauriii: Multi-...

Status: Fixed » Closed (fixed)

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

Chris Burge’s picture

Chris Burge CreditAttribution: Chris Burge at University of Nebraska commented

Good work on fixing the spillover issue. I wish I'd found this issue when I opened #3080698: Multiple select element is inappropriately targeted with CSS in settings tray in Sept. 2019. We still have an issue with color and background-color for <option> in multiple select elements.

How it looks:
screenshot with incorrect option color

How it should look:
screenshot with correct option color

Patch needs reviewed at #3080698: Multiple select element is inappropriately targeted with CSS in settings tray.