Problem/Motivation

See #3278331: [Meta] Tasks to remove Seven from core and move to contrib and #3278212: [Meta] Tasks to deprecate Seven.

Seven will be deprecated in 9.5.x for removal in 10.0.x, this issue should handle the actual removal from core itself.

Steps to reproduce

Proposed resolution

Remaining tasks

  1. The change record for this issue should include a link to recommendations page, https://www.drupal.org/node/3223395#s-MODULE_NAME. (For example, the CR for removing HAL)
  2. Tag this issue 'Needs release note.'
  3. Remove the module ;-).
  4. Remove references from core/phpstan-baseline.neon.
  5. Check for references in @todo.
  6. Handle migration tests.
  7. In all the functional tests in migrate_drupal_ui make sure that MODULE_NAME is not installed. MODULE_NAME should also be removed from the methods getAvailablePaths() and moved to getMissingPaths() in the tests using those methods.

User interface changes

API changes

Data model changes

Release notes snippet

The Seven theme has been removed from Drupal core and is now available as a contributed theme. Sites using Seven should install the contributed version or switch to a different administration theme such as the new core admin theme, Claro.

Issue fork drupal-3304285

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

catch created an issue. See original summary.

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

longwave opened merge request !2645

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs review

First pass, did not regenerate the dictionary yet.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Claro references Seven in a comment, in CSS that supports IE11 - this will be removed in #3254202: Remove IE11 Support from Claro

quietone’s picture

quietone commented
Issue summary: View changes
Parent issue: » #3278331: [Meta] Tasks to remove Seven from core and move to contrib

Parenting, adding remaining tasks.

quietone’s picture

quietone commented
Issue summary: View changes
Issue tags: +Needs release note

Added change record.
I don't expect any changes to migration tests and I see that none are failing in the test results.

I am updating the IS with the above.

quietone’s picture

quietone commented

found two instances of Seven in @todos.

$ grep -riI -A2 --exclude-dir="seven" @todo core | grep -i seven
core/modules/media_library/src/Plugin/Field/FieldWidget/MediaLibraryWidget.php-      // seven_preprocess_fieldset__media_library_widget(), when
core/modules/workspaces/css/workspaces.overview.css:/** @todo Move to Seven theme before Workspaces is marked stable. */

For the first one I did another search, $grep -riI -A2 --exclude-dir="seven" seven_preprocess core and got a screenful of matches.

quietone’s picture

quietone commented
Issue summary: View changes

formatting fix

catch’s picture

catch
he/him
Primary language English
commented

@todo Move to Seven theme before Workspaces is marked stable. */

I think we can just update the @todo to say Claro here. Wouldn't hurt to check if there's an issue to do it.

quietone’s picture

quietone commented

For the first grep result in #8, there is an existing issue, #3156739: Address @todo in claro_preprocess_fieldset__media_library_widget, so we can leave that one alone.

The second one, I did not find a duplicate. #10 is a good solution.

catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes
Issue tags: -Needs release note +10.0.0 release notes

Just committed #3084814: Deprecate Seven theme so this issue is officially unblocked.

There's no strict dependency between this issue and #3304256: Remove Bartik from Drupal core, however there is between #3270899: Remove Color module from core and Bartik removal. Ideally we'd get the Bartik deprecation in asap due to the color module support, so it might be worth unofficially postponing this until the Bartik issue lands to minimise re-rolls in either direction.

Adding a release note.

bbrala’s picture

bbrala
Location Netherlands
commented
Assigned: Unassigned » bbrala
bbrala’s picture

bbrala
Location Netherlands
commented
Assigned: bbrala » Unassigned

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

bbrala’s picture

bbrala
Location Netherlands
commented
Status: Needs review » Needs work

I did a little rebase, mostly just string changes and missing files since bartik is no longer in core.

I notices a few files we missed changing i think:

A reference to a seven hook.
node-edit-form.html.twig: @see seven_form_node_form_alter()

A reference to seven in shortcut.module

shortcut.module:

  // Theme settings are not configuration entities and cannot depend on modules
  // so to set a module-specific setting, we need to set it with logic.
  foreach (['seven', 'claro'] as $theme)

A reference to the testtheme subseven in the dictionary.
distionary.txt: subseven

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review

Rebased again, fixed the build by removing empty files, addressed #16, fixed another few comments.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Needs work

Looks like one more bit of bartik to clean up in the MR.

Checking core/themes/bartik/css/classy/components/messages.css


themes/bartik/css/classy/components/messages.css
 1:1  ✖  Unexpected empty source  no-empty-source

ravi.shankar made their first commit to this issue’s fork.

ravi.shankar’s picture

ravi.shankar commented
Status: Needs work » Needs review

Tried to address comment #18, and removed empty files from the Bartik theme, please review.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Thanks @ravi.shankar.

The test fails are due to some code deleted in WebAssert::buildStatusMessageSelector() which previously referenced Bartik and Seven, but the same applies to Stark, so I reinstated it.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

Except for the one suggestion from Wim, I checked this branch out and did the following:

find . -name '*seven*'
Nothing found!

grep -irn '*seven*' .
Nothing found here either!

That's about as complete a removal from core as I can fathom. So once Wim's suggestion is implemented, I guess this is RTBC.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Rebased and simplified DbUpdateNegotiator::determineActiveTheme().

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs review » Reviewed & tested by the community

I'm not sure what else there is to do here. Let's knock it out.

  • catch committed c7fa15f on 10.1.x 
    Issue #3304285 by longwave, Spokje, bbrala, ravi.shankar, quietone,...

  • catch committed c5b5b27 on 10.0.x 
    Issue #3304285 by longwave, Spokje, bbrala, ravi.shankar, quietone,...
catch’s picture

catch
he/him
Primary language English
commented

Committed/pushed to 10.1.x and cherry-picked to 10.0.x, thanks!

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.