The page title should update both in the top bar in the list. This is a common pattern many tools similar to XB. If we don't show name in the list once it has been changed, it is really hard to find the right page when creating several pages in a single publish, since all of them would show up as "Untitled page".
lauriii → created an issue.
Cross referencing an issue to add support for textarea in the code editor which is likely very similar to this.
Is this arbitrary wait impacting the "Remove" button in the media widget too? I'm running into a problem where the "Remove" button doesn't work on an occasion.
Tested this manually. I can't get this to show the "Published" status which I assume is because of 🐛 Once previewed in XB an entity with no changes will still show up in "Review x changes" Active . Otherwise this seems to work great! 👏
This is almost correct. The library should show components only from the active theme. This is different from the current functionality where it’s showing components from all themes. Based on this, the decision tree would be:
1. Is the component dynamic (consumes implicit inputs/context or has logic)?
YES ⇒ dynamic_components
NO: Go to 2.
2. Is the component provided by a module?
YES ⇒ Go to 2.B
NO ⇒ Go to 3.
2.B I the providing module XB?
YES ⇒ elements
NO⇒ extension_components
3. Is the component provided by the active theme (or its base theme)?
YES ⇒ primary_components
Fallback: ⇒ Component not listed in XB.
Tested this with the hero component from SDDC: https://git.drupalcode.org/project/demo_design_system/-/tree/1.0.x/stars....
Yep, we should just get rid of the module.
Looks like the bug which I reported in #65 where the default image is lost when changing props is still happening: 🐛 Default image is lost after changing props Active .
Cannot reproduce anymore.
Looks like this is working after re-installing. Not sure what was causing this because I didn't change anything about my installation 🤷♂️
It must be possible to provide components with optional image fields without a default value, hence empty examples must be considered a valid schema for optional images.
\Drupal\experience_builder\ComponentMetadataRequirementsChecker::check() only checks for the presence of an examples[0], but it does not verify that it actually complies with the JSON Schema!
But in this case examples[0] doesn't exist: https://git.drupalcode.org/project/demo_design_system/-/blob/1.0.x/stars... 🤔
Looks like a duplicate of 📌 ComponentMetadataRequirementsChecker::check() should validate that the example(s) actually match the JSON schema Active .
lauriii → created an issue. See original summary → .
lauriii → created an issue. See original summary → .
Why would we allow deleting these? If you delete the components, they simply come back after clearing caches. It seems only disabling and enabling make sense 🤔
I'm not sure we should be just deleting the autosave entry when an entity is updated. I realize this is an edge case but this could still result in loss of data for the user. I think in this situation it might make sense to provide the user an option to choose if they want to continue from the autosave state of the latest revision.
Looks like there's test coverage already. The MR needs a rebase still though.
This was already fixed in ✨ Focus mode for global regions Active .
Checked with @bnjmnm and we're good to merge! 🚀 Hopefully the final commit of the week!
I feel pretty strongly that it would be better to be opinionated on the libraries and not allow individual sites to meddle with these. From users perspective, there's a lot of value in having consistency how these are presented to them. For example, this allows us to provide documentation and video materials that are actually meaningful to the end user. If we identify other common needs, we could easily add backend / UI support for additional libraries even if XB doesn't add anything to those libraries by default.
Addressed feedback 🚀
This works great! 👍 Just waiting for one more confirmation to merge.
I know I've written this somewhere but I can't find it anywhere so I'm writing this again here for context. The different places where components will appear in the UI are the following:
- Elements: Provided by Experience Builder
- Extensions: Provided by Modules
- Dynamic Components: Blocks
- Library: SDCs from the Active Theme + JavaScript Code Components
Each of these UIs have categorization within that category for its components. Here are few screenshots of how these might look like:
@larowlan says that @effulgentsia pointed out we also need "Extensions", but … extensions don't come from Component config entities nor ComponentSource plugins, so how does that connect?
I believe this would be referring to the fact that extensions panel will show module provided components in future:
Crediting designers on this.
Crediting the designers who worked on this.
This looks 🔥🔥🔥! Great work @hooroomoo! 👏
A tiny suggestion I have is that could we add a loading animation for the state where the preview is being loaded when opening the code editor?
@wim leers asked me to test this with more permutations to get a sense where we are:
I'm getting this error when I try to place "Container" element from demo design system. It has an optional image without an example.
AssertionError: assert(\is_array($this->value)) in assert() (line 78 of /var/www/html/web/modules/contrib/experience_builder/src/PropSource/FallbackPropSource.php).
Placing the "Hero" component which has an optional image with an example value works but the media library doesn't open when I click "Add media". If I change other props than the image, the default image is lost.
This should work eventually for Nodes (and other content edited in XB). However, XB doesn't currently support anything except pages (except for development). I doubt we would implement the entity creation the same way for other entity types like we did for pages so it seems that trying to fix it right now would be premature. However, If we can easily fix this for other content entity types at once, that's great.
I don't really understand the confirmation panel. What are we trying to warn the user about? I don't remember seeing designs with a confirmation panel. I understand that we have a confirmation panel for deleting a page since it takes place immediately. But since changing home page goes through the review changes I don't see why we need the warning.
Setting a draft page as home page would be fine since it would be published once we have 📌 Create an endpoint to publish all auto-saved entities Active . We need to think about archived content in relation to setting a home page once we implement support for that.
Would be good to create the issues for the XB UI work as well.
This looks like a solid plan. We can do this in XB first but this is probably something core should handle for us. Ideally, even the "bypass node access" wouldn't be able to bypass this but we can leave that to the core issue.
We don't have full designs for this flow but attaching a design that shows how the homepage would look like in the navigator / top bar which addresses most of the concerns. The home icon should appear in the review changes panel too. We could open a follow-up to explore if we need to clarify this action in the review changes panel.
Let's focus on the deletion aspect in 📌 Disallow deleting an XB-enabled content entity if it's currently the homepage Active .
Adding a gif
@balintbrews Added steps to the IS 😊
I believe 📌 Hide "Published" field from Pages Active could use the heuristic defined here for identifying new entities because the pages are created initially as unpublished, but need to be published as part of the "Publish all changes" action.
The styles seem to be loading fine but the dialog buttons are not working. If we're not fixing it here, we'll need a follow-up for that.
But also now that I am thinking about. What happens if you delete the page you set as the home page?
We should not allow deleting the home page. User would have to assign another page as a home page before they could delete it. I'll let you decide if we should implement that here or in another issue.
Did you mean to refer to
✨
The status badge should indicate if there are changes to the page
Active
? I'm not sure I follow how it introduces a requirement for \Drupal\Core\Entity\EntityPublishedInterface? We are using \Drupal\Core\Entity\EntityPublishedInterface in the Page entity type to workaround the entity creation process and there's the archived state but is there something else that introduces a dependency on it?
Agreed that it would be great to place take care of placing the message. I like the proposal to place the message as a fallback on top of the content region. 👍
Yes, we should allow that!
This looks good except Cypress tests are failing. 👍
I think it's fine to punt the archived state to a follow-up issue. I added here for context because it would be good to think how we'll make it happen but we don't really need it right now.
Note that we would only do this for entities that have autosaved changes associated to them.
I think we need to update the UI to clarify this because this is not at all what I expected 😅 I think #5 makes sense as a direction for this based on what this UI actually does.
When new content is created as draft, the content should be marked as published when the "Publish all changes" is pressed but we're creating an unpublished skeleton entity. However, there's also the archived state which I would imagine would use the status field as well, but in that case we'd want them to unarchive before the content would be published. I'm not sure how we'd know in which scenario we're in.
Maybe I don't understand what this setting is doing 🤔 What happens to the regions that are not exposed to Experience Builder? I thought they would be managed by Block Layout but turns out that probably isn't the case.
Here's the same bug reproduced with an article.
This is also not working for me at least when launching from the "Page data" tab:
It also looks like some of the Claro CSS is leaking to the XB UI after opening the media library:
Not able to test with the image component because of 🐛 Adding the Image component results in a state considered invalid Active .
Here's a flowchart that I believe should answer the questions from #3:
- New content is created which has never been published: Draft
- Content has been published and no changes has been made: Published
- Changes have been made to content that was previously published: Changed
- Content that has been published previously but has been since then unpublished: Archived
Instead of using NULL to return all of the image properties in the image field, I opted to use -1 to match the cardinality unlimited constant used in Field Storage.
+1 for using -1.
Allow passing an optional image style to override the configuration provided by the view mode passed to the template. This will invoke createDerivative if the file doesn't exist and includes the itok token.
I'm not 100% convinced we should be handling image styles on this level. The problem we'd run into is adding support for responsive images. This is closely related to ✨ Create global 'image' component roughly based on NextJS's image component Active . I believe we should have an easy way to create responsive images in the context of a component so that I don't have to go to the Drupal UI to create these. If we implement this, then we'd want to always retrieve the source image and applying the image style would happen after running this filter.
Related feature request.
Not mentioned here but we discussed on Slack that it would be simple to create a module for this. We could add that to the CR in case that someone wants to create that.
I noticed this can happen page template config is invalid for one reason or another. This can be reproduced by trying to delete page title block. It was pretty confusing so we should keep this open.
Thank you @anjali rathod! Closing as outdated.
Removing the product review tag based on #38. Removing the subsystem maintainer review tag since removing the field type from UI is much less invasive than moving it to contrib.
I went through different contrib use cases for floats and I didn't really find good examples where float would be used correctly as a bundle field. There are examples of valid use cases where float is used as a base field. This doesn't mean someone couldn't be using it for the right reasons as a bundle field but by nature float is probably something that would be more likely to be used as base field for use cases like computations.
If float field type is a minimal maintenance feature for us, I'm wondering if we should keep float as a field type in core but start by marking it as no_ui: true? A contrib module could be then be added to add it to the Field UI for those that need it there.
+1 for the removal as discussed during the Burgas onsite. The content on this page isn't relevant in the world of CKEditor 5.
Debugged this with @plopesc and this looks like a regression in Chrome in version 133. 😭
WFM
Postponed on 🐛 "Published" checkbox is always checked even if the entity is not Active .
You're right, we've designed the UX to avoid exactly that 🤦♂️ We should avoid reversing that because it will have major consequences on the UX.
Let's simplify this and work for now with the assumption that default cannot be used for required images, that way the field can be left empty in which case it falls back to the default image. I think that's an acceptable state until we've figured out support fo image examples, which would allow marking images as required.
Thank you!
I now realize that I didn't pay attention to the contents of the page. What's interesting is that if I access that page as an anonymous user, I can see the stub page. The stub page should be created as unpublished so that it cannot be accessed.
Adding 🐛 The icon for the more actions button is not visible Active to the must haves since it's a major bug that makes it really hard to find actions besides "Edit".