Problem/Motivation

Per #3605702-30: Remove remaining Gin and Claro implementation names from Default Admin theme and #31:

Looks like follow up work is neede because there are still instances of Claro

(main)$ git grep -ni claro | grep core/themes/default_admin
core/themes/default_admin/css/theme/accent.css:41:[data-admin-focus="claro"] {
core/themes/default_admin/css/theme/accent.pcss.css:37:[data-admin-focus="claro"] {
core/themes/default_admin/images/core/README.md:3:should be removed before moving Claro to Drupal core. See
core/themes/default_admin/images/core/README.md:4:https://www.drupal.org/project/claro/issues/3045216 for details.
core/themes/default_admin/src/Settings.php:295:        'claro' => $this->t('Legacy green'),
core/themes/default_admin/tests/fixtures/implementation-name-allowlist.json:14:    "pattern": "'claro' =>",
core/themes/default_admin/tests/fixtures/implementation-name-allowlist.json:19:    "pattern": "data-admin-focus=\"claro\"",
core/themes/default_admin/tests/fixtures/implementation-name-allowlist.json:24:    "pattern": "data-admin-focus=\"claro\"",
core/themes/default_admin/tests/fixtures/implementation-name-allowlist.json:49:    "pattern": "Claro",
core/themes/default_admin/tests/fixtures/implementation-name-allowlist.json:54:    "pattern": "project/claro",
core/themes/default_admin/tests/fixtures/implementation-name-allowlist.json:104:    "pattern": "(?:gin|claro)",
core/themes/default_admin/tests/fixtures/implementation-name-allowlist.json:109:    "pattern": "(?:Gin|Claro)",
core/themes/default_admin/tests/src/Unit/ImplementationNameTest.php:145:    $name_pattern = '/(?:\b(?:gin|Gin|claro|Claro)\b'
core/themes/default_admin/tests/src/Unit/ImplementationNameTest.php:146:      . '|(?<![A-Za-z])(?:gin|claro)(?=[._-]|[A-Z])'
core/themes/default_admin/tests/src/Unit/ImplementationNameTest.php:147:      . '|(?<![A-Za-z])(?:Gin|Claro)(?=[A-Z]))/';
core/themes/default_admin/tests/src/Unit/ImplementationNameTest.php:166:      if (preg_match('/(?:gin|claro)/i', $file->getFilename())) {

core/themes/default_admin/css/theme/accent.pcss.css - Should the be data-accent.pcss.css
core/themes/default_admin/images/core/README.md - This states the core/themes/default_admin/images/core can be deleted. Is that a followup?

And also instances of gin.

(main)$ grep -iwlR claro core/themes/default_admin
core/themes/default_admin/tests/fixtures/implementation-name-allowlist.json
core/themes/default_admin/tests/src/Unit/ImplementationNameTest.php
core/themes/default_admin/css/theme/accent.pcss.css
core/themes/default_admin/css/theme/accent.css
core/themes/default_admin/src/Settings.php
core/themes/default_admin/images/core/README.md

(main)$ grep -iwlR gin core/themes/default_admin
core/themes/default_admin/tests/fixtures/implementation-name-allowlist.json
core/themes/default_admin/tests/src/Unit/ImplementationNameTest.php
core/themes/default_admin/tests/src/FunctionalJavascript/LegacyStorageTest.php
core/themes/default_admin/tests/src/Functional/AdminTest.php
core/themes/default_admin/migration/js/init.js
core/themes/default_admin/src/Hook/FormHooks.php
core/themes/default_admin/src/Hook/ThemeHooks.php
core/themes/default_admin/src/Settings.php
core/themes/default_admin/config/install/default_admin.settings.yml
core/themes/default_admin/logo.png
core/themes/default_admin/font/inter.woff2

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#11 3623097-nr-bot_ur53t6xh.txt1.08 KBneeds-review-queue-bot

Issue fork drupal-3623097

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

godotislate created an issue. See original summary.

godotislate’s picture

godotislate
he/him
commented
Parent issue: #3605702: Remove remaining Gin and Claro implementation names from Default Admin theme » #3582351: [Meta] Clean up CSS within Admin theme
Related issues: +#3605702: Remove remaining Gin and Claro implementation names from Default Admin theme
godotislate’s picture

godotislate
he/him
commented

I think we can exclude implementation-name-allowlist.json and ImplementationNameTest.php, because those are for testing explicitly that there aren't files with gin or claro in the name.

gábor hojtsy made their first commit to this issue’s fork.

gábor hojtsy opened merge request !17151

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Proposed an MR. Only one image needed to be kept in Default Admin which I moved elsewhere, the other ones I found equivalent images.

LLM was used to help me track these down.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Active » Needs review
jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
commented
Status: Needs review » Reviewed & tested by the community

I've looked through all the changes and they all look correct. Setting to RTBC.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented

Ran visual regression tests. Looks great! Committing in a sec.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented

Getting some failures in precommit hook tests. Merging main and will wait for tests to pass.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new 3623097-nr-bot_ur53t6xh.txt1.08 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

  • mherchel committed 90701646 on main 
    task: #3623097 Remove remaining Gin and Claro implementation names from...
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs work » Patch (to be ported)

Committed and pushed 9070164623d to main. Thanks!

Looks like the issue was with my local Node version.

smustgrave made their first commit to this issue’s fork.

smustgrave opened merge request !17173

smustgrave’s picture

smustgrave commented
Status: Patch (to be ported) » Reviewed & tested by the community

All green backport.

  • godotislate committed b8829bf5 on 11.x 
    task: #3623097 Remove remaining Gin and Claro implementation names from...
godotislate’s picture

godotislate
he/him
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Compared 11.x and main diffs and they look the same.

Committed b8829bf and pushed to 11.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • godotislate committed 31567154 on 11.x 
    task: #3623097 Remove remaining Gin and Claro implementation names from...

  • godotislate committed 67b98c16 on 11.x 
    Revert "task: #3623097 Remove remaining Gin and Claro implementation...
godotislate’s picture

godotislate
he/him
commented

Sorry, forgot a few files in the first 11.x commit, so I reverted, then re-committed everything.

Committed 3156715 and pushed to 11.x. Thanks!