Contrib.social

Refactor olivero.theme as @todo linked issue is closed now #3099026

Open on Drupal.org →
Created on 15 March 2023, almost 2 years ago
Updated 17 March 2023, almost 2 years ago

Problem/Motivation

As discovered in #3123832: [META] Fix @todo items referencing closed issues 📌 [META] Fix @todo items referencing closed issues Active , 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

📌 Task
Status

Needs work

Version

10.1 ✨

Component
Olivero  →

Last updated 12 days ago

Created by

🇮🇳India gauravvvv Delhi, India

Live updates comments and jobs are added and updated live.
  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

Sign in to follow issues

Comments & Activities

  • Issue created by @gauravvvv
  • Status changed to Needs review almost 2 years ago
  • Comment almost 2 years ago →
    🇮🇳India gauravvvv Delhi, India
  • Comment almost 2 years ago →
    🇮🇳India nikhil_110

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

  • Status changed to Needs work almost 2 years ago
  • Comment almost 2 years ago →
    🇺🇸United States smustgrave

    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.

  • Status changed to Needs review almost 2 years ago
  • Comment almost 2 years ago →
    🇮🇳India gauravvvv Delhi, India
  • Assigned to Guru2023
  • Comment almost 2 years ago →
    🇮🇳India Guru2023
  • Issue was unassigned.
  • Status changed to RTBC almost 2 years ago
  • Comment almost 2 years ago →
    🇮🇳India Guru2023

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

  • Status changed to Needs work almost 2 years ago
  • Comment almost 2 years ago →
    🇺🇸United States bnjmnm Ann Arbor, MI

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

  • Status changed to Needs review almost 2 years ago
  • Comment almost 2 years ago →
    🇮🇳India gauravvvv Delhi, India

    Removed the unrelated file, added interdiff for same

  • Status changed to Needs work almost 2 years ago
  • Comment almost 2 years ago →
    🇺🇸United States smustgrave

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

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024