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

Comments

Gauravvvv created an issue. See original summary.

gauravvvv’s picture

gauravvvv commented
Status: Active » Needs review
StatusFileSize
new 3348035-2.patch3.17 KB
nikhil_110’s picture

nikhil_110 commented
StatusFileSize
new Screenshot-3348035-3.png135.54 KB

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

smustgrave’s picture

smustgrave 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 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 commented
Assigned: Unassigned » guru2023
guru2023’s picture

guru2023 commented
Assigned: guru2023 » Unassigned
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new 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
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 commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2_9.txt1.13 KB
new 3348035-9.patch2.04 KB

Removed the unrelated file, added interdiff for same

smustgrave’s picture

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

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

quietone’s picture

quietone commented
Status: Needs work » Postponed

The Olivero theme was approved for removal in #3590816: [policy, no patch] Deprecate Olivero and move to contrib.

This is Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.

The deprecation work is in #3595082: [meta] Tasks to deprecate the Olivero theme and the removal work in #3595085: [meta] Tasks to remove the Olivero theme.