backdrop-issues icon indicating copy to clipboard operation
backdrop-issues copied to clipboard

Multivalue fields with long text elements can break the layout

Open laryn opened this issue 1 year ago • 7 comments

Description of the bug

Multivalue fields with long text elements can break the layout, as reported in these locations:

  • https://github.com/backdrop/backdrop-issues/issues/6364
  • https://github.com/backdrop-contrib/paragraphs/issues/163 (This is in the Paragraphs queue but has been confirmed as a core issue on multivalue fields)

The first issue has a PR that is RTBC and likely to get into 1.27.0. However, @yorkshire-pudding and I have verified that the issue is not specific to CKEditor and is more general.

Cause

See https://github.com/backdrop-contrib/ckeditor5/issues/125#issuecomment-1889609592

Solution

If we can fix the wider issue, then perhaps we can remove the CKE5-specific fix and avoid adding the same fix in multiple places. Adding this in system.theme.css right after the offending section solves both cases. Does that seem appropriate?

.field-multiple-table tr.odd .form-item,
.field-multiple-table tr.even .form-item {
  white-space: normal;
}

laryn avatar Jan 15 '24 15:01 laryn

Can we update this issue to elaborate on how to recreate the issue when not inserting long text into ckeditor 5? I can't recreate the bug in core, not with inserting long text into a text field with no editor, nor when putting long text into the help field in the field config.

herbdool avatar Jan 15 '24 17:01 herbdool

@herbdool Thanks for looking. It looks like it may require a separate module to interact with it, which wraps the field with the long description in a multivalue table. e.g. Paragraphs, Multifield, Field Collection, custom code... If so, does that mean we leave it to each of those to fix if desired or is it still worth considering a broader fix in core?

laryn avatar Jan 15 '24 21:01 laryn

Looking at the offending CSS, I'm not sure what it is actually attempting to do. Perhaps there are places in core where a checkbox or radio button is within a table and the label should not be wrapped? I might be inclined to remove the white-space: nowrap; rule and fix any places in core that would be affected (though I can't find any at first look).

quicksketch avatar Jan 17 '24 22:01 quicksketch

I had the same thought... it feels extremely heavy handed to add a general no-wrap at that level.

laryn avatar Jan 18 '24 02:01 laryn

Yes, let's get a PR for that. Removing that rule altogether and test for any breakages.

herbdool avatar Apr 11 '24 22:04 herbdool

@laryn I've added a PR here to just remove that line. We seem to be in agreement on that. Testers can see if it breaks any layouts in core.

I took a look at the history of that line. It's really old. I could only see back 14 years to when it was moved to the current file, but couldn't find anything before that linked to an issue to explain why it was added.

herbdool avatar Jun 03 '24 16:06 herbdool

Perhaps there are places in core where a checkbox or radio button is within a table and the label should not be wrapped? I might be inclined to remove the white-space: nowrap; rule and fix any places in core that would be affected (though I can't find any at first look).

Testers can see if it breaks any layouts in core. ...

I vaguely recall something related with that and mobile/narrow screens. I'll have to test.

klonos avatar Jun 05 '24 04:06 klonos