🇮🇳India @nayana_mvr

Addressed review comments.

Resolved merge conflict and build issues. Should we remove core/themes/stable9/css/node/node.module.css also?

nayana_mvr → made their first commit to this issue’s fork.

Moving to Needs Review for maintainers feedback.

Can someone please confirm if this ticket is still relevant? I found another ticket https://www.drupal.org/project/drupal/issues/3396523 📌 node.module.css is obsolete and can be removed Active in which the file node.module.css will be removed since it is no longer used. This is the same file which is being updated in the last patch submitted here #3 📌 Make two column node CSS reusable Active . Also, noticed that classes in Block layout UI are different from the one attached in last comment.

Thanks! @larowlan for the suggestions. I have implemented those changes except flex-grow since form element seems to be getting adjusted correctly even without flex-grow because of width:100%. Below are the changes done in this commit:-

1. Added display: flex to parent element that contains the two prefix/ suffix spans and the form element
2. Reverted width: 100% of form element.
3. Added spacing for prefix and suffix referring to the style used for suffix in form.css for screen width above 601px. As I'm not sure why that breakpoint was used in form.css, I added them without breakpoints in table.css

After the above changes this is how the fields looks like and I think it's is much better than previous implementation:

Updating IS screenshots as well.
Please review.

I just tried a mock up of how the form field will look like in the user profile.

This require more work as it will change all the font size of admin ui text based on the value selected in user profile.

Hi @johnwebdev, could you please provide some steps to reproduce this issue?

Regarding #30, I'm able to reproduce the point 1 (Please see the screen recording. This is after applying the current MR changes).

I think it is because of max-width calculation. For .media-library-item__attributes, a max-width: calc(100% - 10px); is used but I'm not sure why it’s added. Removing that or setting that to 100% fixes this issue. I can implement this change if someone can confirm about this solution.

As for point 2 in #30, I'm unable to reproduce that issue. Can be verified in the screen recording attached.

nayana_mvr → changed the visibility of the branch 3338309-media-library-grid to hidden.

Regarding which MR to review:- As per #23 🐛 Media library grid item label sticks out, media alignment should adhere to positioning standards RTBC ,

..addition of margin: 0.1rem; in media-library-item__attributes does not make much difference to the solution, rather it misaligns the title towards the top right corner.

MR!9369 has the correct changes. So I'm hiding the branch of MR!8547 as i'm unable to close that MR.

There is one observation :- the issue mentioned in the ticket occurs when Autocomplete widget is used in the form display whether the field is limited or unlimited. If Autocomplete (Tags style) widget is used, then this issue doesn’t occur.
The screenshot of 'tags' was added just to show how an entity reference field with unlimited value + Autocomplete (Tags style) widget looks like in the form. As you can see in the screenshot, the width of that field is not set to 100%. When Autocomplete widget is used, the field appear as a table and width: 100% was set to the input elements and I think it was done so that it will cover the entire row of the table. Attaching a screen recording of entity reference field without prefix/suffix and toggling width:100%.

I'm not sure if we can remove width completely here. Better to write tests.

Updated IS with before and after screenshots.

The changes in the patch are already implemented in core as part of the ticket https://www.drupal.org/project/drupal/issues/3298580 🐛 Claro details component does not have the right class Fixed . But still I'm able to reproduce this issue. Attaching a screen recording for reference.

In the template, the class form-item__error-message is added using below code:-

{% if errors %}
        <div class="form-item form-item__error-message">
          {{ errors }}
        </div>
      {% endif %}

But variable errors is always returning null. I don't know from where that value is coming. Need to debug more on this. I'm removing the proposed solution given as that change is already implemented.

nayana_mvr → made their first commit to this issue’s fork.

Pipeline was not shown in the Merge Request and there was an option to create pipeline. After clicking that, the .gitlab-ci.yml file changes got committed to the MR. Not sure why that happened and why that file was missing in the first place. I tried creating a new branch but that also doesn't have .gitlab-ci.yml. Can somebody please look into it? Sorry for the inconvenience caused.

nayana_mvr → made their first commit to this issue’s fork.

As per feedback comment, I have done the changes using php. Please check if this is fine. Changes in the UI are same as shown in previous screenshots. So not attaching them again.

@sagarmohite0031 Please change the admin theme to Olivero and then verify again.

From the attached recording, I can see you’re still using Claro theme as admin theme.

Please find the before and after MR screenshots of comment section of an article.

Before:

After:

I tried adding a Date and Mail id field to the comment entity and this is how it looks like after the MR changes:

I have re-rolled the patch for Drupal 11.x and raised MR. Please review.
There are two more CSS instance other than Claro where this update may require i.e.,core/profiles/demo_umami/themes/umami/css/classy/components/inline-form.csscore/themes/starterkit_theme/css/components/inline-form.css. But I couldn't find any separate file for field-ui in those 2 themes. So didn't make any changes there.

nayana_mvr → made their first commit to this issue’s fork.

I have re-rolled the patch for D11 and raised MR. Updated TODO items in the Issue Summary. Please check if it's fine.

nayana_mvr → made their first commit to this issue’s fork.

Marking it as 'Needs Review' to get feedback from maintainer.

Verified this issue in Drupal 10.4.x as Entity embed module is not compatible with D11. The css code added in #2 patch has been already implemented for the Claro (core admin theme) so this issue is not reproducible with Claro theme (current core admin theme). As for Olivero (core default theme), the dropdown is still clipped with a scroll in the dialog but I think it is fine because in Olivero 'Next' button is coming in the left bottom. If dropdown is not clipped, it will overlap on the 'Next' button. I'm not sure whether we need to make any changes at system level now since already theme level fix is implemented for Claro. Attaching screenshots for reference.

Claro:

Olivero:

I have added the colon ':' as a translatable string in twig template core/themes/claro/templates/classy/field/field.html.twig and created an MR against 11.x branch. Attaching before and after screenshots of a Greek page after translating ':' to an empty string for Greek language in UI translation for reference.

Before:

After:

Please review.

@sagarmohite0031 As mentioned in the steps to reproduce,

Open node edit form with Oliviero

So change the admin theme to Olivero in /admin/appearanceand then check the node edit page.

nayana_mvr → made their first commit to this issue’s fork.

Verified the changes in MR!8893 in Drupal version 11.x and can confirm that it fixes the issue. The position remained centre aligned even while scrolling the navigation bar (please refer the screen recording). Attaching before and after screenshots as well. Need RTBC+1

Before:

After:

Updated IS with latest screenshots.

Can someone please specify the steps again to reproduce this issue. Because I'm unable reproduce the issue with the steps mentioned in the IS. Steps followed:-

1. Installed and enabled Webform and Webform Template modules.
2. Navigated to /admin/structure/webform/templates.
3. Scrolled the table.
4. Table header is positioned at the top on scroll as sticky header.

Attaching a screen recording for reference. Also, noticed that the table is not within the .tabs in this case. Please correct me if I'm not correctly understanding the issue.

Is there any other scenario where this issue can be reproduced in D11? Because Webform module is not compatible with 11.x yet.

Actually the temporary solution which is mentioned in the ticket has been implemented as part of another ticket which is in RTBC now. Attaching that ticket here. Maybe that will solve both the issues.

The proposed solution says

Add a preference for minimum size to the user page /user/[userNumber]/edit

But for which all text should that font size be applied? Is it like, if there are texts with font size below the specified font size in the user page, then those text should use the font size given in the user page?
Another apporoach I would propose is to create a checkbox in user page enabling which will use --font-size-s in places where --font-size-xs and --font-size-xxs are currently used.

I have re-rolled the patch D11 incorporating all the style changes from last patch into Claro's print.css and modified some classes as per Claro theme. There are some phpunit file changes in the patch which I don't think is relevant to this ticket, so didn't include that in the re-roll. Had to remove the following code which was mentioned in the description

@page {
  margin: 0.5cm;
}

from the MR as it was throwing lint error. Also, I couldn't verify that change in my local i.e., it didn't made any difference to page margin in print mode.
I have a doubt also reg. this ticket. Don't we need a print style for Olivero theme because that is the default theme that replaced Bartik theme right? Currently I couldn't find any print style file for Olivero. Please let me know if anything needs to be done in Olivero. MR is raised, please review.

nayana_mvr → made their first commit to this issue’s fork.

This issue is not reproducible in Drupal core version 11.x. Tried with single select field and multiple select field. Please find the screenshots attached.

But I noticed that last few texts are getting trimmed in the list if it is more that the width of the dropdown. I feel atleast some ellipses should be shown to indicate that there are more text.

Patch didn't get attached in previous comment. Attaching it again.

I tried to create 2 different patches: one for vertical tab (core module) other for horizontal tab (field group contrib module version 8.x-3.x). This is the solution I mentioned in my previous comment which may not be a correct method. I haven’t tested it with all the cases listed in #44 🐛 Field Groups marked as required are missing red asterisk Needs work but this works for Tabs. Attaching screenshots for reference.

Vertical:

Horizontal:

I verified the issue with patches of #89 and #90 with 2 scenarios : Tabs vertical and Tabs horizontal. Both the patches fixes the issue for Tabs vertical but for Tabs horizontal, 2 asterisks are displayed. Please see the screenshots below:-

Vertical:

Horizontal:

I noticed that for vertical tab, template is rendered from core module and for horizontal tab template is rendered from contrib module field group that has <span class="required-mark"></span> with ::afterwhich results in 2 asterisks.

Vertical:

Horizontal:

Changed version by mistake. Changing it back to 11.x.

Created MR with the new approach and updated IS with complete template. Please review.

PHPUnit Functional Javascript failures shown in the MR are not reproducible in local. Command used vendor/bin/phpunit --configuration phpunit.xml {filename}. Attaching screenshots as evidence. Please let me know if there is any other way to reproduce it in local. Moving this ticket back to Needs Work so that someone else can also verify it once.

Verified the patch on D11 and the changes applied cleanly so re-roll was not required. I have created an MR against 11.x branch with the changes in #3. Attaching before and after screenshots of D11 for reference. Please review.

Before:

After:

nayana_mvr → made their first commit to this issue’s fork.

Attaching few issue links which I referred while looking into this ticket. Please feel free to remove if it's not relevant.

I have re-rolled the MR to D11 but there are UI issues as shown in below screenshot.

The class names .input-group and .input-group__text are not getting added in the expected elements when compared to the HTML structure shown in http://getbootstrap.com/components/#input-groups. Also, as per my understanding, input-group can only be used if the theme uses Bootstrap style (Please correct me if I'm wrong). In D11 Claro theme doesn't use Bootstrap but there are many contrib themes such as Radix, Bootstrap, etc which can be used to achieve this.

I tried a different approach for the UI part where I could make the input field and search button inline to each other. Screenshot attached below.

Attaching both re-rolled patch and patch with new approach here.

This issue seems to be an old one. Currently in D11, there exists one filter functionality in module extend page so do we still need to work this issue? If not please close this ticket. Moving to Needs Review for feedback.

nayana_mvr → made their first commit to this issue’s fork.

As per comment in the parent ticket https://www.drupal.org/project/drupal/issues/2489394#comment-9956761 →

it's confusing that we have search-result__info, search-result__snippet, and search-result__snippet-info, all as different things.

So, do we need to change all the three class names or can we just change .search-result__snippet-info to something which doesn't contain the words info or snippet. I think .search-result__content used in the MR is a good example or else please suggest new class names.
After the current MR changes, this is how it looks like:-

nayana_mvr → made their first commit to this issue’s fork.

There was a pipeline error as shown below,

It says .pcss.css file from line no. 273 is not updated. This happens when .css (compiled) file corresponding to .pcss.css file is not updated with the changes. I have faced similar issue before. That’s why I tried the command yarn run build:css in core folder and found that the .css files corresponding to the .pcss.css files shown in the error are not pushed to the branch.

This issue is related to CkEditor field with unlimited values right? But after applying the changes of MR! 5728, it is affecting the CkEditor field with limited values also as mentioned in #3 🐛 CKEditor 5 toolbar items of multi-value field (typically Paragraphs) overflowing on narrow viewports and overlapping with node form's sidebar on wide viewports Needs work

The solution of removing "nowrap" doesn't look like a good approach to fix the issue as it's breaking the default behavior of the editor which wraps all the items into a popup when there are too many items.

Before:

After:

With 100% resolution

With 90% resolution

I think this is because of the following code:-

@media screen and (max-width: 90rem) {
  .ck .ck-editor__top .ck.ck-toolbar.ck-toolbar_grouping > .ck-toolbar__items {
    flex-wrap: wrap;
  }
}

Is it a new approach we are going to follow?

This is how it looks like in layout builder basic block.

Re-rolled patch for D11 and raised MR. Keeping it in 'Needs Work' status as there are Nightwatch test failure issue in the pipeline.

Obseration:-
These changes need to be implemented in field group also right? I noticed that the same classes are used in field group also.

nayana_mvr → made their first commit to this issue’s fork.

I have removed the unused code and the comment and verified that it is not affecting the existing layout builder style. Please review.

Verified the issue mentioned in the ticket description in Drupal version 11.x and can confirm that it is not reproducible. Please see the attached screenshot for reference.

So I think the the code with @todo comment can be removed.

I think the comment

/**
* Chromium and Webkit do not yet support flow relative logical properties,
* such as float: inline-end. However, PostCSS Logical does not compile this
* value, so we accommodate by not using these.
*
* @see https://caniuse.com/mdn-css_properties_clear_flow_relative_values
* @see https://github.com/csstools/postcss-plugins/issues/632
*/

added as part of the ticket https://www.drupal.org/project/drupal/issues/3313146 🐛 PostCSS Logical not transpiling flow relative properties (e.g. float: inline-start) which are not supported by Chrome and Safari Fixed is no longer required since the issue (https://github.com/csstools/postcss-plugins/issues/632) mentioned in that comment is already closed.

nayana_mvr → made their first commit to this issue’s fork.

Fixed pipeline issues and updated Issue summary. Please review.

I have re-rolled the patch for D11 but there are some build issue so keeping it in 'Needs Work'. I have done some additional changes in template and styles to make the UI same as in module listing page. Attaching a screen recording to show how the current changes look like.

nayana_mvr → made their first commit to this issue’s fork.

Verified the changes on Drupal version 11.x and the changes are applied cleanly. Attaching before and after screenshots for reference. Need RTBC+1

Before:

After:

This issue is not reproducible in Drupal 11.x. verified it with frosh Drupal 11.x setup. Attaching SS for reference. I have mentioned the same in my previous comment #49 🐛 active-link.js throws JS error if query string parameter contains a single quote Needs work . I think this ticket can be closed since it is already fixed along with https://www.drupal.org/project/drupal/issues/3464340 🐛 JS errors from Drupal.behaviors.activeLinks (... is not a valid selector) Needs work . I moved it to Needs Review so that someone else can also verify it once.

Attaching screenshots fas evidence.

I think the fix for the ticket mentioned in #47 🐛 active-link.js throws JS error if query string parameter contains a single quote Needs work fixes the issue mentioned in this ticket as well. I applied the changes of that ticket in Drupal 11.0.x and it fixed the js error mentioned here. I didn't verify that initially that’s why I added a commit into the MR here. Maybe someone else can also verify it once so moving it to Needs Review.

nayana_mvr → made their first commit to this issue’s fork.

Attaching screen recording for reference.

This issue is not reproducible in Drupal versions 10.4.x and 11.x. Verified it by adding entity reference media field in content type and paragraph. Attaching screenshots for reference. Also, I didn't get the point that this occurs in dev site and in live site it is working. Please let me know if there is any specific step to be followed other than the steps mentioned in the IS to reproduce this issue in vanilla Drupal setup.

I'm unable to reproduce this issue in D11 local setup navigation module enabled. I tried with CSP(Content Security Policy) module and Seckit (Security Kit) module. Couldn’t find any module named nginx rules. Is there any specific configuration to be done in the CSP or Seckit module to get that error? Also, does the error occur in login page itself or any of the admin page?

I believe adding !important in code is not a good practice. There are inline styles used in ck editor, so it won't be easy to override them using css without !important unless we increase css selector specificity. I have tried a different approach by changing the max-width property value through js. Currently max-width of ck-dropdown__panel is always set to 60vw. I have added a code to dynamically change that value based on toolbar panel width. Some update in alignment may also require, but please check if this approach is useful.

I'm unable to add the remote repo for this issue in my local. Getting the below error. SO I have created a patch. Also, attaching before and after screenshots.

user/drupal11 % git remote add drupal-3475842 git@git.drupal.org:issue/drupal-3475842.git
git fetch drupal-3475842
error: remote drupal-3475842 already exists.
remote: 
remote: ========================================================================
remote: 
remote: A repository for this project does not exist yet.
remote: 
remote: ========================================================================
remote: 
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

This issue exists in Drupal 11.x version. So as per the comment here https://www.drupal.org/project/drupal/issues/3471686#comment-15754893 🐛 CSS "form-item__label" inconsistent on node edit forms Needs review

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

changing the version to 11.x

nayana_mvr → changed the visibility of the branch 3475842-overflow-toolbar-hidden to hidden.

@finnsky Sorry for the confusion. Since there could be more changes, I thought creating patch would be better than directly committing to the MR. I have now committed the changes to MR!9516. Please check.

nayana_mvr → changed the visibility of the branch 3408500-move-save-button-to-topbar to active.

nayana_mvr → changed the visibility of the branch 3408500-move-save-button-to-topbar to hidden.

@finnsky I have attempted your suggestions and created a patch. Following are the changes done based on the suggestions:-

• Created new buttons when 'type=submit' which are 'Save' and 'Preview' buttons in case of node edit form. For 'Delete' action, I cloned the element as it is as I was not sure if it needs to be changed to <button>.
• Copied all attributes except 'class' from original buttons to new buttons and added new classes 'toolbar-button toolbar-button--primary' to the new buttons since both buttons have 'type=submit' (Referred class names from https://thunderous-treacle-600c5c.netlify.app/?path=/story/page--basic).
• Placed the new buttons and Delete link in the top-bar.

This is how the buttons look like now:-

Regarding the original buttons, should we just hide them from the UI or remove them completely once new buttons are created?

This is just an attempt based on my understanding. As mentioned in previous comment, this may require more research on Drupal core forms.

Hi @jessebaker, I tried to reproduce this issue in Drupal cms, but I'm getting some error. Steps followed:-

1. Installed Drupal CMS (starshot) and Experience Builder module and enabled it.
2. Created a new field in Basic page content type of field type Experience Builder.
3. In the manage dispaly, the new field was in disabled section.
4. Dragged the field to enabled section and tried to save configuration.
5. Got the following error.

Please let me know if I'm missing some steps here. Also please update the Issue Summary with steps to reproduce.

Verified this issue on Drupal version 11.x, Project Browser version version 1.0.x, Gin version 8.x-3.x and it is not reproducible. Steps followed:-

1. Installed Project Browser module in D11 setup.
2. Changed admin theme to Gin and enable dark mode.
3. Reloaded the page /admin/modules/browse and verified that the whole page is in dark mode.

Screenshot attached for reference.

Please let me know if this needs to be tested in any other versions. Or maybe someone else can also test so keeping it in Needs Review for now.

I found one similar issue https://www.drupal.org/project/drupal/issues/3100133 🐛 Ajax replace inside a modal causes the current field to lose focus. Needs work . As per a comment in that ticket,

Some of the underlying logic was changed in #3397785: Dialog focus is returned to dialog instead of first tabbable element on dialog AJAX update

And I think the changes in https://www.drupal.org/project/drupal/issues/3397785 🐛 Dialog focus is returned to dialog instead of first tabbable element on dialog AJAX update Fixed caused this new issue. I found one patch in the comment section https://www.drupal.org/project/drupal/issues/3397785#comment-15665315 🐛 Dialog focus is returned to dialog instead of first tabbable element on dialog AJAX update Fixed which fixes this issue. I'm raising it as an MR here. But I think those changes were made for D10 when the 'Add field' section of paragraphs were in a modal box. In D11, it is a separate page. So do we really need those code which is introducing the new focus issues in layout builder?

As per comment https://www.drupal.org/project/drupal/issues/3471686#comment-15754893 🐛 CSS "form-item__label" inconsistent on node edit forms Needs review

Changes are made on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

So changing the version to 11.x after verifying that this issue is reproducible in 11.x version.

The original issue mentioned in this ticket

Dropbutton field pushed out of the table if no label set

is not reproducible in Drupal version 11.x. Attaching screenshots for reference.

But as mentioned in #52 🐛 Dropbutton field pushed out of the table if no label set Needs work , there are UI issues when a field which cannot be redirected is added in the Global dropdown (SS can be referred in #52).
In the Global dropdown configuration, all the fields which are added in the view are listed down as options for global dropdown items. Also, there is a help text saying "Fields to be included as links." which doesn't make sense because fields such as Content Type, Status, Updated, etc doesn't have any link to redirect, so why do we have to show those fields as an option for global dropdown? Also Operations field which is already a dropdown should not be allowed as an item in another dropdown right? (I'm checking this in /admin/structure/views/view/content)

Please correct me if I'm getting this wrong. If we need to work on dropdown style, then I think it should be tracked in a different ticket since the original issue of this ticket is already fixed in D11. Marking this as Needs Review for feedback from others.

I'm able to reproduce this issue on Drupal 11.x and the changes in MR!5511 fixes that. Regarding the comment

/**
 * Fix duplicate border caused by normalize.css adding border-bottom without
 * removing the text-decoration.
 */

It refers to the file core/themes/stable9/css/core/assets/vendor/normalize-css/normalize.css where the following style is written for abbr[title] :-

/**
 * 1. Remove the bottom border in Chrome 57-
 * 2. Add the correct text decoration in Chrome, Edge, IE, Opera, and Safari.
 */

abbr[title] {
  border-bottom: none; /* 1 */
  text-decoration: underline; /* 2 */
  text-decoration: underline dotted; /* 2 */
}

I verified this in Chrome and safari, but there are no duplicate borders in any of them after this fix. Not sure whether there was any code change in core/themes/stable9/css/core/assets/vendor/normalize-css/normalize.css later.

Noticed that text-decoration works differently in both browsers. Attaching the before and after(chrome and safari) screenshots for reference.

Thanks @finnsky for the information. I will be careful about that in future. I have update the code using vanilla js. Kindly review.

The issue is reproducible in Drupal 11.x version. I have created an MR with the proposed solution. I didn't change

display: block;
 margin-block: var(--sp0-5);

because I think it is as per Olivero theme style and doesn’t require any change. I have used variable --line-height-s for line-height property since the variable used in Claro is specific to Claro theme and it won't work for Olivero. Kindly review the changes.

nayana_mvr → made their first commit to this issue’s fork.

@finnsky No specific reason. jQuery is my primary language. If required, I will re-write it in vanilla js.

Changes in MR!169 are not getting applied in Drupal 11.x. Getting the following error:-

Checking patch js/form-actions.js...
Checking patch navigation.libraries.yml...
error: navigation.libraries.yml: No such file or directory
Checking patch js/form-actions.js...

Tried to apply the changes of MR!7943. Changes applied cleanly but getting console error.

I have created a new MR!9516 incorporating review comments of MR!169. I have placed all the action buttons in top bar otherwise it will be confusing for the editors imo. This is currently working for node edit form, media edit form and taxonomy term edit form.

Regarding,

the user generally expects to start at the top of the form and work their way down until they get to the bottom where they expect to find the submit button.

I think placing the action buttons in top bar is useful so that editor can save the form immediately after editing a field instead of scrolling down the whole form (E.g., if editor needs to edit just the first field value.). Also, top bar is currently available only on entity edit forms. When user is creating a new node/ term/ media, the save button will be in the bottom itself.

Kindly review the changes. Thanks.

nayana_mvr → made their first commit to this issue’s fork.