Overview

In #3506467: Make Media Library widget work from "page data" (content entity form) we enable claro as the admin theme for media library tests.
Wim asked if we should be doing it for all e2e/tests that use XbTestSetup.

Proposed resolution

Consider enabling claro as the admin theme in XbTestSetup

User interface changes

CommentFileSizeAuthor
#10 Screenshot 2025-03-24 at 2.11.09 PM.png42.42 KBbnjmnm
#10 Screenshot 2025-03-24 at 2.13.36 PM.png93.3 KBbnjmnm

Issue fork experience_builder-3514551

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

larowlan created an issue. See original summary.

omkar-pd made their first commit to this issue’s fork.

omkar-pd opened merge request !810

omkar-pd’s picture

omkar-pd commented
Status: Active » Needs review

wim leers made their first commit to this issue’s fork.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: Unassigned » bnjmnm
Status: Needs review » Reviewed & tested by the community
Issue tags: +e2e

Thoughts, @bnjmnm? 😊

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Consider enabling claro as the admin theme in XbTestSetup » Install Claro as the admin theme in XbTestSetup, for all end-to-end tests

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

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Assigned: bnjmnm » Unassigned

This seems fine - AFAIK the only motivations behind not doing this initially is no longer relevant at this stage in the project. After merging 0.x with this I did encounter some problems that I addressed largely in XBTemplateRenderer, and also extended one of the e2e timeouts so it spanned the wait of the autosave interval + a little additional time.

Needs ui codeowner approval.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
StatusFileSize
new Screenshot 2025-03-24 at 2.13.36 PM.png93.3 KB
new Screenshot 2025-03-24 at 2.11.09 PM.png42.42 KB

This also needed a manual run of the tests currently skipped for headless: prop-types.cy.js and autocomplete-props.cy.js:

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: Unassigned » larowlan

@larowlan recently touched some of the changes @bnjmnm just made here in #3506467: Make Media Library widget work from "page data" (content entity form), so I'd like to have him explicitly review this too. 🙏

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Needs work

Needs reroll and I think there are some changes we've already made.
I was also expecting that there'd be a few e2e tests we no longer need to manually enable claro for - which should speed things up.
Sorry I didn't realise this was assigned to me.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: larowlan » Unassigned
Issue tags: +Novice

#12 sounds like something someone unfamiliar with the XB codebase can pull off :)

meghasharma changed the visibility of the branch 3514551-consider-enabling-claro to hidden.

meghasharma changed the visibility of the branch 0.x to hidden.

omkar-pd changed the visibility of the branch 3514551-consider-enabling-claro to active.

meghasharma changed the visibility of the branch 3514551-consider-enabling-claro to hidden.

meghasharma changed the visibility of the branch 3514551-consider-enabling-claro to active.

meghasharma’s picture

meghasharma commented

Reverted all the previous commit changes from here:- https://git.drupalcode.org/project/experience_builder/-/commit/3adb350b7...

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented

The current MR is more unnecessary changes than necessary ones. It might be easier to start a new one.

Also, any e2e tests enabling Claro as the admin theme should have that line removed since it is now doing that by default.

meghasharma’s picture

meghasharma commented

Rebased the branch and setting the admin theme to Claro came as incoming changes.
please check

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

The new MR makes no changes at all, other than whitespace? 😅 I bet you pushed the wrong commit 😁

meghasharma’s picture

meghasharma commented
Status: Needs work » Needs review

#12 Rebased the branch, yes there are changes already merged in 0.x
only needed to do e2e tests we no longer need to manually enable claro, have done in MR
Please review

meghasharma’s picture

meghasharma commented

#22 Yes, because Install and set Claro as the admin theme in XbTestSetup changes already merged in 0.x main branch. may it's done in other issue.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Install Claro as the admin theme in XbTestSetup, for all end-to-end tests » XbTestSetup now installs Claro as the default admin theme, update all end-to-end tests to not manually install it anymore
Priority: Normal » Minor
Status: Needs review » Reviewed & tested by the community
Issue tags: +clean-up
Related issues: +#3512860: Multi-valued fields' widget should fully work in the content entity form

Ah, yes, #3512860: Multi-valued fields' widget should fully work in the content entity form apparently did that!

This MR is still worth landing though: it's a nice bit of clean-up.

Thank you! 😊

  • wim leers committed e9903240 on 0.x authored by omkar-pd 
    Issue #3514551 by meghasharma, bnjmnm, omkar-pd, wim leers, larowlan:...
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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