[PP-1] Make "page data" tab in right sidebar work

📌 HTTP API: new /xb/api/entity-form/{entity_type}/{entity}/{entity_form_mode} route to load form for editing entity fields (meta + non-meta) Fixed is in.

I created an MR with a good starting point for someone to pick up - the form is now rendering in the "Page Data" tab, and using React.

There's plenty to do in addition to me getting the form to appear there. There are fields there we don't want, some of the code used to get this working might be repeating logic that could be reused, the current rendering is too wide for the available width, etc etc.

bnjmnm → closed merge request !356

Even though the form is marked as #is_xb in the initial code by @bnjmnm, no element inside is getting that attribute, so we're not getting them mapped to the JSX components. I managed to find out why.

Marking all children of an element if it has #is_xb is supposed to happen in \Drupal\experience_builder\EventSubscriber\XbMainContentViewSubscriber. This event subscriber is subscribed to the KernelEvents::VIEW event. That event is not getting triggered when a controller returns a JsonResponse. Which is what \Drupal\experience_builder\Controller\EntityFormController (added in 📌 HTTP API: new /xb/api/entity-form/{entity_type}/{entity}/{entity_form_mode} route to load form for editing entity fields (meta + non-meta) Fixed ) does.

Would it be okay to update \Drupal\experience_builder\Controller\EntityFormController to return a render array instead? That would match the behavior of \Drupal\experience_builder\Form\ComponentPropsForm::buildForm. This simple change would immediately solve the mapping problem and unblock this issue. (I verified this with a quick test.)

(My gut feel is that long term we may want a JsonResponse for both controllers so it's more intuitive what's happening even just by glancing at the HTTP response. Not sure if you agree, but either way, that could be a new issue if desired? 😇)

I'm not particularly attached to either return type and don't recall why the props form was not a JSON response. It sounds like it's not a huge effort to switch it to a render array, and switching the other response to JsonResponse also seems OK. For this issue, I'd personally recommend whichever switch requires the least amount of effort, and if it happens to be the Render Array approach we can have a followup to look into making them both JsonResponse

#5: nice start! 🤩🚀

#9: Hm, yes, that was an oversight 🙈 It really should have

    # The XB equivalent of requesting an AJAX response using `?_wrapper_format`.
    # @see Drupal.Ajax.AJAX_REQUEST_PARAMETER
    # @see \Drupal\Core\Ajax\AjaxResponse
    # @see \Drupal\experience_builder\Render\MainContent\XBEndpointRenderer
    _wrapper_format: 'xb_endpoint'

just like the experience_builder.api.component_props_form route definition.

Attached is what I think all you should need to do to A) make it not return JSON, B) make it match exactly the thoughtful principles that @bnjmnm and I brought to the ComponentPropsForm. (That means I disagree with @bnjmnm in #10: going either way does not make sense to me, we should follow the pre-existing pattern, so we can reuse all the infrastructure we already added previously.)
Sorry for the confusion! 🙈

#11: Works like a charm! 😊 What I asked for ++.

Most of the form is now rendered as React components. A lot more to do, but we're off to a good start!

We need 🐛 Unable to scroll component props form Needs review to make the sidebar usable. Not marking this issue as postponed on it just yet, there's plenty to do even without 🐛 Unable to scroll component props form Needs review . I'll be working on these two in parallel.

🥳

🐛 Unable to scroll component props form Needs review is in :)

@bnjmnm, I would appreciate your input on a few questions I have.

1. Both \Drupal\experience_builder\Controller\EntityFormController::form and \Drupal\experience_builder\Form\ComponentPropsForm::buildForm render their responses using the current Drupal theme. This means that the theme assets are added, including its stylesheets, which may contain CSS rules with high specificity (e.g. selectors targeting HTML tags directly). This became very visible and problematic as I started mapping checkboxes to a React component: I found myself writing CSS reset code for those. Have you thought about this situation? Could we switch the theme to e.g. Stark in those controllers? Or maybe you already have a much better idea? Knowing the general direction here would help me decide what is reasonable to aim for with my styling code in the scope of the current issue.
2. Can you please confirm that State API is NOT supposed to be working yet for this form? E.g. “Provide a menu link” checkbox.
3. Can you please confirm that autocomplete inputs are NOT supposed to be working yet for this form? E.g. “Tags” field.

#16:

1. 
   Are you sure? 🤔 They should be rendered using stark, see \Drupal\experience_builder\Theme\XBThemeNegotiator::$routes, added in 🐛 Changing default theme can break HTML forms generated by XB API routes: always negotiate the Stark theme Fixed .

Assigned to @bnjmnm for the remaining 2 questions.

#17: Great, so Stark is already the negotiated theme for those routes. I started to investigate how I'm still seeing Olivero's CSS added in the XB app, then after a clear cache etc. it's gone. Now the task is to figure out how to reproduce it being added. 🙂

Can you please confirm that State API is NOT supposed to be working yet for this form? E.g. “Provide a menu link” checkbox.

Can you please confirm that autocomplete inputs are NOT supposed to be working yet for this form? E.g. “Tags” field.

During early evaluation/proof-of-concept stages we confirmed that these functionalities can work, but this was before we had solid tests in place where we could ensure that functionatly continuesto work. It's very possible that something changed in the past several months that changed what is needed to get those working. I don't think this issue scope needs to be concerned with any functionality we have not confirmed to be working with e2e tests. I created 📌 Confirm Semi-coupled form elements can work with autocomplete Active and 📌 Confirm Semi-coupled form elements can work with State API visibility Active

That's helpful, thanks, @bnjmnm!

This is now ready for review. I think the best person to do the first round would be @bnjmnm. 😊

The MR description outlines what has been done and what is out of scope for this particular issue.

Gitlab does not list me as a codeowner that can approve this, which I assume is due to me being responsible for some of the earlier commits on the MR, but all the changes since mine are good by me.

Anything I spotted that I'd typically point out in a review happens to be something that either will be refactored out of existence within a week or two, or related to interactions that don't have formal requirements, so it's better to have this committed so it can help those requirements be discussed.

I approved the remaining thing that needed codeowner approval. I tried to merge this, but it said a pipeline is failing. I suspect due to 🌱 [Proposal] A different approach to the iFrame preview interactions Active not being merged into this fork yet (or this fork rebased on top of that), but not sure.

I need to slightly adjust a test now that the other issue landed.

Here is where we ended up with this issue:

🤩 @balintbrews GIFs never disappoint! 😁

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