backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 39 forks source link

Multivalue fields with long text elements can break the layout #6365

Open laryn opened 8 months ago

laryn commented 8 months ago

Description of the bug

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

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;
}
herbdool commented 8 months ago

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.

laryn commented 8 months ago

@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?

quicksketch commented 7 months ago

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).

laryn commented 7 months ago

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

herbdool commented 5 months ago

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

herbdool commented 3 months ago

@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.

klonos commented 3 months ago

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.