Closed (fixed)
Project:
Experience Builder
Version:
0.x-dev
Component:
Semi-Coupled theme engine
Priority:
Critical
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
2 Oct 2024 at 14:52 UTC
Updated:
17 Feb 2025 at 10:50 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
johnwebdev commentedComment #3
wim leersThanks, @johnwebdev, appreciate that 😊
Comment #6
deepakkm commentedComment #7
wim leers@deepakkm is no longer working on XB.
Comment #8
wim leers… and AFAICT this will allow us to delete the very existence of the
#is_xbrender array property — because this MR will make it possible to simply always use thexb_starktheme for certain routes.And with
#is_xbobsolete, we'll be able to delete ALL infrastructure around it (that sets it when appropriate):\Drupal\experience_builder\RenderArrayXB::markXB()\Drupal\experience_builder\RenderArrayXB::findAndMarkXB()\Drupal\experience_builder\EventSubscriber\XbMainContentViewSubscriberexperience_builder_preprocess_form_element()experience_builder_form_component_props_form_alter()… and
experience_builder_theme_suggestions_alter()will be able to remove its firstif-statement.Am I missing something, or can you confirm this, @bnjmnm? 😄
Comment #10
wim leers🤩
Comment #11
wim leersThis MR looks great! (And once the commented out lines in the
.modulefile are deleted, it'll passphpcstoo).Just one small remark, really.
And … we of course need to get the tests passing! :D
Comment #12
bnjmnmA nice bonus is that the Media Library widget is also rendered with xb_stark
Although XB stark defaults to using Semi Coupled and rendering with React, the MR includes Twig templates that ensure Media Library is still rendered with Twig. This was added to address an issue where
core/drupal.dialog.ajaxneeds the completed HTML earlier than Semi coupled can (currently) provide it. If we really want to render the Media Library widget in React I'm sure this can be done, but it might be worth keeping as is since it's the format the Media Library UI is known to work well with, and there's no need for it to communicate with redux.This may open options up to more easily address #3471978: Make the Media Library dialog look like the admin theme without materially affecting anything outside of the dialog (possibly use an iframe for CSS isolation) if we want XB specific styles instead of using the admin theme.
Comment #13
bnjmnmComment #15
balintbrewsChanges were introduced upstream by #3469235: Make "page data" tab in right sidebar work that needed to be adjusted to this MR. Needs work:
\Drupal\Tests\experience_builder\Functional\EntityFormControllerTest::assertFormResponseneeds to be updated.Comment #16
balintbrewsI fixed the problems I mentioned in #15, then reviewed all code changes, and approved the MR.
I have three questions/asks.
xb_stark.info.yml.0332d920? There might be a nicer way to do it.includeand refer to the original Twig templates that way in ourprocess_as_regular_twigtemplate overrides. Would we like to do that as part of this issue, or in a follow-up?Comment #17
bnjmnm. Good call, done!
It's good.
Thanks for the reminder! With that change this is an MR that reduces the total LOC by 139!
Just need a few additional codeowner signoffs.
Comment #18
wim leers#12: Nice! 👏
#17:
😮
This looks great, but there's some loose ends on the docs side. I'd like to see those resolved before this lands, because otherwise the burden of fully documenting how this works would shift to #3446434: Document "Semi-Coupled Theme Engine" and "Redux-integrated field widgets" components.
Comment #19
bnjmnmFeedback addressed
Comment #20
bnjmnmComment #21
wim leersLet's ship this! 🚢
Comment #22
wim leersEnd-to-end tests failed randomly, differently: run 1 vs run 2.
⇒ Merging!
Comment #24
wim leersThis:
Comment #28
wim leersWhile blogging about this, retro-actively crediting @larowlan for raising #3467499 and @effulgentsia for the fundamental idea implemented here, which he described in #3467499-13: Use a custom serialization format instead of theming to render the edit form to the intermediary representation that XB's React code consumes! 👏