Long string breaks the layout of Claro

The provided patch adds changes to the theming of Claro.

A patch is provided which adds theming changes to Claro theme.

I can confirm this bug on Drupal 10.2 alpha1, testing with Umami.
Pasting a very long string, the body-field moves right almost completely outside viewport.
Uploading a video to demonstrate. I will test the patch against dev branch.

Yes I also confirm this bug was on Drupal. 10.1.6. I have applied patch #5 which solved this bug. I have attached screenshot before the patch and after patch for reference.

So moving this issue to RBTC +1

The patch changes the UX significantly for large screen and is out of scope for this issue. Very wide textareas are usually considered bad UX.
I working on this at the moment at Helsinki contrib event and my current goal is to fix the bug without changing the layout as a whole.

Think you win most random bug of the week haha. Definitely was able to confirm this issue using a generated string of 2000.
MR 5327 does address the problem. Hiding patches for clarity.

Tested the Merge Request and it seems to do the trick. I agree with @simohell that changing the grid layout on core theme template might have even further bad repercussions. The MR just handles the width's aspect and that should be the way forward.

I posted one minor comment in the MR 😊

Quite right. Missed that one, but now it should be ready.

Functionality not changed since previous review

I'm not sure reducing the width to 52rem is right, on a wide screen it makes it quite narrow? I wanted to add that I was testing this issue but using the patch in 3381219 which seemed to work well for me. And it looks like these two will conflict.

I've been just comparing the solution in this MR against the work in #3381219 🐛 Layout Issues with Claro theme Needs work and while it does resolve the issue with the main column expanding, it also introduces a fixed width when the browser viewport is between 976px and 1278px wide. On narrower viewports, the user must scroll to the right to view and edit the secondary column. I think we should consider this a regression, since the prior float-based layout was fully responsive and compressed the main column. I've confirmed that the scrolling appears in multiple browser engines.

I don't know, maybe I'm biased. But I prefer this approach from #3381219 🐛 Layout Issues with Claro theme Needs work

Ok. I updated the CSS so, that this fix does not create regression to Firefox and Safari.

Firefox and Safari continue to have certain clipping in some viewport widths, but not more than with the current production version. FF and Safari both also benefit from this fix even if other add/edit form layout issues still affect those browser.

I think this small fix is ready to be merged and the other browser specific issues could be handled in 🐛 Layout Issues with Claro theme Needs work .

Ok. I updated the CSS so, that this fix does not create regression to Firefox and Safari.

If you are referring to my comment in #19 🐛 Long string breaks the layout of Claro Fixed , I am testing in Brave 1.60.114 Chromium: 119.0.6045.124, and am still seeing the same horizontal scrollbar in other browsers. I was not describing a cross-browser issue, but a hard-coded width issue that results in scrolling on any browser viewport width between 976px and 1278px wide.

The current MR does not add any hard-coded widths, so maybe you are testing an older commit?

width: 100%;
max-width: 52rem;

Perhaps the old CSS is cached in your setup. Or you have some custom plugin or forced CSS that overlaps here.

With an extremely long string the width may increase for Firefox and Safari but without the change the whole form is pushed away from viewport. Current dev branch does have certain viewport widths where some browsers clip the sidebar a bit, but that is not caused by this MR.

Ah, sorry, I described that incorrectly @simohell. The problem with the horizontal scrolling on narrower viewports is caused by grid-template-columns. That issue is noted and addressed in this patch and comment:
https://www.drupal.org/project/drupal/issues/3381219#comment-15238921 🐛 Layout Issues with Claro theme Needs work

Ah. But it's not a regression by this issue, so this would be improvement as such. I noticed that you updated the other MR so I'll check that one and compare results. This one fixes one single bug, the other one maybe more. Changing only one thing was an idea to get it merged quickly but let's see.

It looks like the fix for current core layout was simpler than I thought.
I managed to test this right, all that was needed was setting a minimun value for main column:
not grid-template-columns: 3fr minmax(22.5rem, 1fr);
but instead grid-template-columns: minmax(0, 3fr) minmax(22.5rem, 1fr);

I just tested the latest change in the MR and it works great for me at wide (1920px) and narrow (977px) viewport widths in MacOS Brave (Chromium), Firefox and Safari. Combine that with the feedback in https://www.drupal.org/project/drupal/issues/3381219#comment-15321780 🐛 Layout Issues with Claro theme Needs work and I think we can update the status to RTBC. Nice work @simohell!

lauriii → closed merge request !5327

Committed 5c2760e and pushed to 11.x. Also cherry-picked to 10.2.x. Thanks!

Created a patch from the accepted solution for anyone, like me, who needs to use a fixed patch file.

@keszthelyi it's a bit hidden, but you can get the patch directly from the MR clicking the text "plain diff"'

@simohell yes, I'm aware of that, but our CI doesn't allow MR patches (not even on commits). I could have used a local patch, but I know there are others in the same situation, so maybe it will be useful for others.

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

I'm testing a Drupal 10.3.0 upgrade and seeing this issue again. It looks like a regression.

Looks like it was reverted as part of https://www.drupal.org/project/drupal/issues/2519362 📌 Redesign the menu link add form to be less overwhelming Fixed .

Thanks @amanire

I am seeing the same or similar on 10.3

Ok what I am seeing is a bit confusing

In 10.2.x we had node-add.css

see https://github.com/drupal/drupal/tree/10.2.7/core/themes/claro/css/layout

which had the following:

@media (min-width: 61rem) {
  .layout-node-form {
    display: grid;
    grid-template-rows: auto 1fr;
    grid-template-columns: minmax(0, 3fr) minmax(22.5rem, 1fr);
    gap: var(--space-l);
  }

So this was applied to .layout-node-form but layout-node-form appears to have changed to .layout-form

So previously in

https://github.com/drupal/drupal/tree/b06bf11bfb1cf4bffa74710cd2ff1b1dd6...

we seemed to inherit from starter kit

https://github.com/drupal/drupal/blob/b06bf11bfb1cf4bffa74710cd2ff1b1dd6...

Now the we have form-two-columns.html.twig in claro

e.g.

https://github.com/drupal/drupal/blob/b06bf11bfb1cf4bffa74710cd2ff1b1dd6...

The markup is slightly different.

Also, node-add.css has been removed and it looks to me as if the new css has now been moved to form-two-columns.css

e.g.

https://github.com/drupal/drupal/blob/b06bf11bfb1cf4bffa74710cd2ff1b1dd6...

previously this was in node-add.css

So, basically the i fix looks like the same as before, but applied to layout-form rather than layout-node-form

e.g.

https://github.com/drupal/drupal/blob/10.2.7/core/themes/claro/css/layou...

It looks to me as if this has been removed in 10.3

As the info is here on this issue, it might make sense to reopen this issue.

Certainly this regression is a direct result of https://www.drupal.org/project/drupal/issues/2519362 📌 Redesign the menu link add form to be less overwhelming Fixed

Alternatively, we could create a new issue as the fix will be slightly different. i.e. the element we are targeting has changed its class name attribute and also changed where the css is kept.

Patch works for me without issue.

I have also opened issue with https://github.com/cweagans/composer-patches/issues/579

This is useful if you have patches for both core and contrib. There is a bug in composer-patches that prevents you from applying patches with different patch levels i.e. core (level 2) and contrib (level 1) without specifying without using the -vvv flag and having to step through patches with different patch levels.

Really not sure why this was not picked up in any regression testing. It seems that using the node edit form is quite a common requirement to me.

I asked @laurii and @xjm in slack admin UI about due process to fix this. Based on Lauri's the answer I will open a new issue.

The new issue to re-fix this is ready for review.

This looks like it has been fixed in 10.3.2

One less patch 😀

Thank you.