Problem/Motivation

Toolbar will either need to refactor or include the library itself.
#3623843: Remove Backbone.js and Underscore.js, not used in core anymore

See how tour did it here: #3558002: Update backbone.js and move off of core

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork toolbar-3624031

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

nicxvan created an issue. See original summary.

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

smustgrave opened merge request !4

smustgrave opened merge request !5

smustgrave changed the visibility of the branch 3624031-core-no-longer to hidden.

smustgrave’s picture

smustgrave commented

For a stable release this should be the last ticket needed. Then I’d start a 3.x branch for any future development

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

smustgrave’s picture

smustgrave commented

On my phone but you can drop the twig pin that should be fixed in core now.

dydave’s picture

dydave commented
Version: 1.x-dev » 2.x-dev

Looks like the tests are failing due to the missing claro theme and workspace module:

There was 1 error:
1) Drupal\Tests\toolbar\Functional\ToolbarClaroOverridesTest::testClaroAssets
Drupal\Core\Extension\Exception\UnknownExtensionException: Unknown themes: claro.
/builds/project/toolbar/web/core/lib/Drupal/Core/Extension/ThemeInstaller.php:66
/builds/project/toolbar/web/core/lib/Drupal/Core/Extension/ThemeInstaller.php:143
/builds/project/toolbar/tests/src/Functional/ToolbarClaroOverridesTest.php:54
--
There was 1 failure:
1) Drupal\Tests\toolbar\FunctionalJavascript\workspaces\WorkspaceTest::testToolbarSwitcherDynamicPageCache
Behat\Mink\Exception\ExpectationException: Current response header "X-Drupal-Dynamic-Cache" is "", but "MISS" expected.
/builds/project/toolbar/vendor/behat/mink/src/WebAssert.php:888
/builds/project/toolbar/vendor/behat/mink/src/WebAssert.php:180
/builds/project/toolbar/web/core/tests/Drupal/Tests/WebAssert.php:979
/builds/project/toolbar/tests/src/FunctionalJavascript/workspaces/WorkspaceTest.php:135
ERRORS!
Tests: 42, Assertions: 470, Errors: 1, Failures: 1.

How do you think we should approach these issues?

OK dropped the twig pin 👌

nicxvan’s picture

nicxvan commented

Claro I assume you can put in require dev right?

Not sure about the workspace failure.

dydave’s picture

dydave commented

Added drupal/claro in composer next major job... the error has changed now:

There were 2 failures:
1) Drupal\Tests\toolbar\Functional\ToolbarClaroOverridesTest::testClaroAssets
Behat\Mink\Exception\ExpectationException: Current response status code is 500, but 200 expected.
/builds/project/toolbar/vendor/behat/mink/src/WebAssert.php:888
/builds/project/toolbar/vendor/behat/mink/src/WebAssert.php:145
/builds/project/toolbar/tests/src/Functional/ToolbarClaroOverridesTest.php:118

2) Drupal\Tests\toolbar\FunctionalJavascript\workspaces\WorkspaceTest::testToolbarSwitcherDynamicPageCache
Behat\Mink\Exception\ExpectationException: Current response header "X-Drupal-Dynamic-Cache" is "", but "MISS" expected.
/builds/project/toolbar/vendor/behat/mink/src/WebAssert.php:888
/builds/project/toolbar/vendor/behat/mink/src/WebAssert.php:180
/builds/project/toolbar/web/core/tests/Drupal/Tests/WebAssert.php:979
/builds/project/toolbar/tests/src/FunctionalJavascript/workspaces/WorkspaceTest.php:135
FAILURES!
Tests: 42, Assertions: 490, Failures: 2.

I'll have to look closer at the issues locally when I get some time to test.

slasher13’s picture

slasher13
Primary language German
commented

Claro will be removed in Drupal 12 see https://www.drupal.org/node/3623824

dydave’s picture

dydave commented
Status: Active » Needs review

OK, all tests are now passing 🟢

Added 2 commits:
1 - To fix the phpunit tests for D12, due to a remaining static file path reference to 'core/themes/claro/templates/navigation' and explicitly enable the 'dynamic_page_cache' module for another test.
2 - Cleaned up the Gitlab CI configuration and fixed a deprecation message in one of the tests.
Unpinned constraints in composer.json file.

I've tested this locally as well with Drupal "12.0-dev" and everything seemed to work fine.

Moving this to Needs review as an attempt to get more reviews and testing feedback.

Thanks in advance!

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Say ship it

nicxvan’s picture

nicxvan commented

@slasher13 you are correct, just like toolbar, that's why we needed to add it to composer for the tests.