Problem/Motivation

As discovered in #3123832: [META] Fix @todo items referencing closed issues, there is a @todo in core/themes/olivero/olivero.theme which has a link to an issue that is fixed. As the parent issue is fixed, we can now refactor the code as done in claro.theme file.

Here's the @todo:

// @todo change this after https://www.drupal.org/node/3099026 has landed.
    $variables['table']['#header'][0]['data'] = [
      '#type' => 'html_tag',
      '#tag' => 'h4',
      '#value' => $variables['element']['#title'],
      '#attributes' => $header_attributes,
    ];

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#9 3348035-9.patch2.04 KBGauravvvv
#9 interdiff-2_9.txt1.13 KBGauravvvv
#7 after-apply-patch#2.png200.8 KBGuru2023
#3 Screenshot-3348035-3.png135.54 KBNikhil_110
#2 3348035-2.patch3.17 KBGauravvvv
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gauravvvv created an issue. See original summary.

Gauravvvv’s picture

Gauravvvv CreditAttribution: Gauravvvv at Axelerant for Drupal India Association commented
Status: Active » Needs review
FileSize
3348035-2.patch3.17 KB
Nikhil_110’s picture

Nikhil_110 CreditAttribution: Nikhil_110 at Srijan | A Material+ Company commented
FileSize
Screenshot-3348035-3.png135.54 KB

Patch #2 has been successfully applied.. I have attached the screenshot..

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative, +Needs issue summary update

Seems to have a number out of scope changes outside of removing a todo.

If more is being fixed here it should be added to the issue summary.

Gauravvvv’s picture

Gauravvvv CreditAttribution: Gauravvvv at Axelerant for Drupal India Association commented
Title: Remove @todo-comment linking to closed issue in olivero.theme » Refactor olivero.theme as @todo linked issue is closed now #3099026
Issue summary: View changes
Status: Needs work » Needs review
Parent issue: » #3099026: Claro's preprocessing of field multiple value form's table header cell removes potential changes by others
Guru2023’s picture

Guru2023 CreditAttribution: Guru2023 at Srijan | A Material+ Company commented
Assigned: Unassigned » Guru2023
Guru2023’s picture

Guru2023 CreditAttribution: Guru2023 at Srijan | A Material+ Company commented
Assigned: Guru2023 » Unassigned
Status: Needs review » Reviewed & tested by the community
FileSize
after-apply-patch#2.png200.8 KB

I have reviewed patch #2 and it has been successfully applied. Please see attached screenshot for reference .

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
CreditAttribution: bnjmnm at Acquia commented
Status: Reviewed & tested by the community » Needs work

@Nikhil_110 there's no value in showing screenshots of code or patches applying. We have automated processes (notice the green box that says "pass") that confirms a patch applies

@Guru2023 there's no value in showing a screenshot of changed code. The patch shows the code getting changed, the testbot lets us know it works.

+++ b/core/themes/olivero/css/components/form.pcss.css
@@ -16,8 +16,11 @@
-.form-item__label--multiple-value-form {
+.form-item__label--multiple-value-form,
+.field-multiple-table .field-label h4.label {
+  display: inline-block;
   margin-block: 0;
+  vertical-align: middle;
   font-size: inherit;
   font-weight: inherit;
   line-height: inherit;

Can we get an explanation in the issue summary what this change is for. It seems like the preprocess is removing the label class so I'm not sure why a style is being added that targets it. An explanation may be all that is needed.

Gauravvvv’s picture

Gauravvvv CreditAttribution: Gauravvvv at Axelerant for Drupal India Association commented
Status: Needs work » Needs review
FileSize
interdiff-2_9.txt1.13 KB
3348035-9.patch2.04 KB

Removed the unrelated file, added interdiff for same

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs review » Needs work

Going to say this needs an issue summary update still. What's the proposed solution?

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.