- 🇺🇸United States justcaldwell
The approach in #16 worked well for us — thanks @aludescher!
One note: be sure to add
'converterPriority' => 'high'
to your new options, e.g.:$options[] = [ 'model' => 'headingFancy', 'view' => [ 'name' => 'h2', 'classes' => 'fancy', ], 'title' => t('Heading 2 (fancy)'), 'class' => 'ck-heading_heading2_fancy', 'converterPriority' => 'high', ];
Without that, added options likely won't still be selected on subsequent edits of the content. Standard options that operate on the same element (in this case 'Heading 2') will take precedence.
- 🇺🇸United States justcaldwell
@taote you might be missing a use statement for CKEditor5PluginDefinition at the top of your module file:
use Drupal\ckeditor5\Plugin\CKEditor5PluginDefinition;
- 🇬🇧United Kingdom AaronMcHale Edinburgh, Scotland
benjifisher → credited AaronMcHale → .
- 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
Follow-ups:
- #19.1: 📌 [later phase] [PP-1] When the field type for a PropShape changes, the Content Creator must be able to upgrade Postponed (and I realized it had a blocker itself: ✨ [later phase] PropShapes' JSON schema must match exactly with FieldType's storage format — what if you want to use another FieldType? Active ), and then the 3 at the bottom of #17:
- 📌 [PP-1] Auto-generate Component config entities for all discovered SDCs that meet XB's minimum criteria Active
- 📌 [PP-2] Remove Component config entity's add/edit form Postponed
-
📌
[PP-1] Introduce `FieldTypeStorageForJsonSchemaDefinition` plugin type
Postponed
Bonus follow-ups: 📌 Component config entity should validate that the SDC actually (still) exists Active + 📌 [PP-2] Add ComponentAuditabilityTest Active .
I'll continue working on this MR on Monday. Now: time for blog posts :)
- 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
- 🇺🇸United States douggreen Winchester, VA
I still get a PHP warning in AttributeArray (line 79) because the original input array is statically cached in ViewExecutable::getExposedInput() on line 726, which happens before the data is validated. We need to validate the data before it is saved.
- Issue created by @benjifisher
- 🇫🇷France nod_ Lille
Committed and pushed 17aee79159 to 11.x and ae41c62daf to 11.0.x and 8912d59338 to 10.4.x and da2452190d to 10.3.x. Thanks!
- 🇺🇸United States dblanken
dblanken → changed the visibility of the branch 10.1.x to active.
- 🇬🇧United Kingdom scott_euser
Thanks for the work on this one.
I agree on the 3rd level, e.g. Taxonomy Overview is not possible to navigate to, you need to first go to e.g. 'Add vocabulary' /admin/structure/taxonomy/add then use the breadcrumb to get to the overview at /admin/structure/taxonomy
I would also like to second this one:
The title within the drawer wasn't considered click-able. It looked like a header with no affordance being a clickable link.
We had clients with the same issue, it was not obvious that they could get at the /admin/people list from this 'People' so they believed they only had access to create new users, but not access a list of existing users to make sure their colleagues could log in:
A possible solution to that one (which could apply to the lower level as well) is to separate the menu item link from the ability to navigate to its child items:
This seems to match the advice in https://infinum.com/blog/website-navigation-dropdown-menus/ which shows that its 'Version 2' which has both the top level link clickable + the overview link within the sub-navigation as being the quickest to achieve the task + the least margin of error for users.
A less clear UX article on it from NN hints at this too in item 12, but the separation only becomes clear when you actually visit the example they point to
- 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
Discussed #17 with @lauriii. His replies:
- Yes, but needs follow-up to devise a UX to allow existing
StaticPropSource
s using a previous field type+widget to update to the new one. - Yes
- Yes
He agreed with the 3 follow-ups proposed at the bottom of #17.
We also discussed #18, and he prefers the "alternatively" — so repurposed that 👍
P.S.: Note that this also blocks 📌 Create a component for testing form backend + frontend integration Active .
- Yes, but needs follow-up to devise a UX to allow existing
- 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
Oh, and once this lands, 📌 [PP-1] Media Library integration Postponed becomes trivially unblocked: it'll only require:
- new configuration to allow declaring a preference for
image
vsmedia
(which oddly would not be possible through a newexperience_builder.settings.yml
config, only through a new config entity, because simple config cannot have module nor config dependencies) - updating
\Drupal\experience_builder\SdcPropJsonSchemaType::findFieldTypeStorage()
to respect that configuration
Those 2 things would be in an MR for that issue which I could start and then the front-end team could continue.
Alternatively, I could repurpose ✨ Hardcode image props to use the media widget temporarily Active for that, so that #3454173 can stay focused on the bigger picture, of making that media library widget actually work (well).
- new configuration to allow declaring a preference for
- 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
The
\Drupal\experience_builder\Form\ComponentEditForm
UX became simpler:Discussion with @lauriii
Yesterday morning, prior to writing #15, @lauriii and I met and discussed this problem space in detail.
The key outcomes of that discussion:
- XB will require that an SDC provides an example value for it to be surfaced in XB. IOW: the
examples
array must not be empty, and XB will use the first one. ⇒ Removes the need for 25% of what theComponent
config entity provides. - @lauriii was heavily leaning towards "no UI" and "logic with an alter hook".
- Tricky: complex shapes, like
json-schema-definitions://experience_builder.module/image
— those would need a plugin system to provide the appropriate field type + field storage.
This issue: current MR state and current consequences
This issue proves it's possible to generate the default
field_type
,expression
andfield_widget
value for each prop. ⇒ Removes the need for 75% of what theComponent
config entity provides.This issue did implement it all as logic (i.e. with nothing stored as a config entity — see my comment #15 for why that turned out to be trickier than I thought when I created this issue), with a TODO for an alter hook.
As demonstrated in #16, this means the scope for 📌 [PP-1] Add support for matching SDC prop shape: {type: string, enum: …} Postponed changes to only having to worry about matching existing field instances that can be adapted to match the SDC prop's enum values.
Questions for @lauriii prior to landing this MR
- Is it acceptable for the logic that determines which field type + widget to use to change and hence newly created instances of this component to potentially use a different field type than the pre-existing component instances?
- SDC supports default
*.component.yml
-defined values. But that won't work for e.g. a default image. Is it okay for XB to layer on support for that by auto-discovering apropname.(gif|png|jpg|…)
file in the SDC's directory? 🤔 (Will add this to 📌 [SPIKE] Comprehensive plan for integrating with SDC Needs work , to potentially add that to upstream SDC.) - Is it acceptable that the default value sourced from a component's
examples
cannot be overridden on a per-site basis?
I think the answer to the first question is "yes". (Note that every stored
StaticPropSource
is self-contained: the user will continue to be able to edit it, even if it's a completely different field type.)If the answer to the second and third questions is "yes", then we can in principle remove the
Component
config entity. But that would introduce a huge risk: it'd make exporting/sharing configuration (in the future: 🌱 [later phase] [META] 7. Content type templates — aka "default layouts" — affects the tree+props data model Active , but now:config/optional/field.field.node.article.field_xb_demo.yml
, which has config dependencies on theComponent
config entities it uses) brittle.Therefore I think it'd be preferable to keep
Component
config entities under the hood — without showing any of that in the UI. I think that would also keep the door open for supporting per-site default values in the future, plus it will likely help with 🌱 [META] Support component types other than SDC Active .If you agree, then I'll create three follow-up issues:
- one that generates
Component
config entities for all discovered SDCs 1) upon installing XB, 2) upon installing a new module or theme - one to remove
\Drupal\experience_builder\Form\ComponentEditForm
after that auto-generation is happening - one for computing the appropriate field type + storage for
$ref
-defined prop shapes
- XB will require that an SDC provides an example value for it to be surfaced in XB. IOW: the
- 🇧🇾Belarus beloglazov91
The updated version of patch compatible with 2.1.0-beta1.
- 🇳🇿New Zealand quietone New Zealand
I read the IS summary, the comments and the MR. The proposed resolution is out of dateAll questions are answered and the threads in the MR are correctly resolved.
I then applied the diff and tested. This works as expected and is an improvement!
I do think there follow up work.
- Add this feature to media files as well.
- The warning message uses the string 'It is advised' which is the first occurrence in core for a warning message. Because of that I think that the usability folks should review the string.
- The first time I read "It is advised to store file uploads for contact forms as private files. You can configure this in settings.php" I read the first as something to action so I tried to. Of course, the option is not available and I knew that but I still tried! So, again, I think the a usability review would help.
I think the above items can be done in followups. I am adding the tag for that.
I finally read the test. Is there a reason it is only testing 1 code path?
- 🇳🇿New Zealand quietone New Zealand
I read the IS and comments. This is changing the UI, so adding tag.
I applied the diff and it works as expected, after clearing caches. I see that the after image in the issue summary does not match what I see in testing. In my testing '(navigation)' is '(Navigation)'. The latter is consistent with the other usages in the block list. And because of the consistency I am not tagging for a UX review.
I think this is a bug because it affects the functionality of the site for admins and developers.
Leaving at RTBC
- 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
I started with a test that gathers all unique SDC prop schemas. Then I started writing logic to automatically suggest the sane core-provided field type choice.
In doing so, I started treading on the territory of #3456008 — and I believe that we now should not do that issue anymore, at least not in its original scope: 📌 [PP-1] Add support for matching SDC prop shape: {type: string, enum: …} Postponed .
I am now thinking that not using a config entity type might actually be the only viable choice, because the config entity being proposed here only is viable if there is a good way to transform a given
SDC prop schema
into aPropShapeInput config entity ID
— because that's how we could achieve sane dependencies.The only reason that worked in the original proposal, is because HEAD does not yet support the
{type: string, enum: …}
use case, which is AFAICT widespread among SDCs. Once that is considered, the only way to generate a config entity ID is by hashing the enum.The issue title + summary need a drastic update. See the new test class, which has 3 test methods that build on top of each other. I'll continue this MR tomorrow and will update this issue as well as 🌱 [META] Early phase back-end work coordination Active .
- @wim-leers opened merge request.
- 🇳🇿New Zealand quietone New Zealand
The two changes in the patch have been fixed elsewhere
- update.theme_install was removed in 📌 Remove adding an extension via a URL Postponed
- update.theme_update was changed to what is in the patch 🐛 Broken Breadcrumb on Appearance tab. Fixed
This is now outdated.
- 🇬🇧United Kingdom longwave UK
I updated the link in #11. I think this will get slightly more use than "Data model changes" which I can't remember the last time I saw filled in.
I also find it somewhat funny that we are known as a CMS for structured content and yet here we are, writing structured content into a free text body field, when we raise issues against our own software - if we used proper fields we could have help text, show different fields for bugs vs tasks, etc.
- 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
@lauriii is not yet convinced, but he's not available to discuss it further, so to A) get this moving, B) ground the conversation with @lauriii more, getting started on this nonetheless.
- 🇬🇧United Kingdom welly
This was a long time coming! I remember starting work on this issue 8 years ago :) Glad to see it finally get over the line!
- 🇬🇧United Kingdom alexpott 🇪🇺🌍
Do we need to update https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-... → to?
One thing that makes me hesitate on this change though is that we need a plan for how this information is going to be used. If it's just just going to sit in the issue summary then not many people are going to see it and the value in writing it is minimal. I realise the issue summary says that this will help the initiative and that's a good thing, but I think the process needs to be a bit more apparent. For example, as this is a new section I think the issue summary template could link to initiative and be more explicit about what is expected - other people are not going to know what to do and whether it is relevant. On a large number of issues, i.e. nearly all bug fixes, this section should be removed or N/a'd.
- 🇬🇧United Kingdom catch
Note there's now a stable trash module in contrib https://www.drupal.org/project/trash →
I haven't personally used it, but I believe it is the result of @amateescu's comment in #50.
- 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
Updated #3462310-10: Component edit form: make form elements match design → and I see @bnjmnm already started working on ✨ Contextual form values need to be integrated with Redux Active 🚀
- 🇳🇿New Zealand quietone New Zealand
@mxr576, thanks for the snippet. It was very clear, thanks! I tweaked it a bit. There is an issue for the 10.4 beta release notes 📌 10.4.0-beta1 release notes Postponed . You may want to add that snippet to those sometime. I have updated the tags.
- 🇦🇺Australia pameeela
Just came to create this issue if it didn't already exist :) This drives me nuts!
Surely this change won't break any tests?
- 🇩🇰Denmark ressa Copenhagen
Yet another little paper cut bites the dust, thanks everyone who made this happen!
- 🇪🇸Spain rcodina
@smustgrave @alexpott Pipeline errors gone. Thanks for your tips!
- 🇬🇧United Kingdom longwave UK
Committed to 11.1.x and backported to 10.4.x as a usability bug fix. Not eligible for 11.0.x or 10.3.x as it changes the user interface.
Committed and pushed afdee270cb to 11.x and c0a5216300 to 10.4.x. Thanks!
The suggested followup already exists, will un-postpone it after submitting this comment.
-
longwave →
committed afdee270 on 11.x
Issue #2502637 by bnjmnm, shumer, smustgrave, cilefen, Wim Leers,...
-
longwave →
committed afdee270 on 11.x
-
longwave →
committed c0a52163 on 10.4.x
Issue #2502637 by bnjmnm, shumer, smustgrave, cilefen, Wim Leers,...
-
longwave →
committed c0a52163 on 10.4.x
-
bnjmnm →
committed 9b971118 on 0.x authored by
Wim Leers →
Issue #3461422 by Wim Leers, bnjmnm, tedbow: Evolve component instance...
-
bnjmnm →
committed 9b971118 on 0.x authored by
Wim Leers →
- 🇺🇸United States michaellander
I'm still a bit conflicted, in that I think we should just allow modules to extend on the layout builder usage, and also allow users to add blocks to the navigation footer. However, I realize there's some hesitancy here, so we still should consider giving some means for modules to extend these areas.
- 🇺🇸United States smustgrave
Tested MR 5685
Verified I was getting the same results as @quietone
On a standard profile install
I was able to toggle the default Full HTML text format to disabled/enabled@steveoriol would see #157 and #161 to see why it could be an issue to delete a text format.
- 🇺🇸United States smustgrave
Think @alexpott answered this for you but there are now recipes in the repo with some image config that need the same fix you did for
core/modules/media_library/tests/modules/media_library_test/config/install/field.field.media.type_four.field_media_extra_image.yml - 🇺🇸United States dead_arm
"Introduced terminology" is now a section included in the Issue summary template, https://www.drupal.org/project/drupal/issues/3449694 📌 Add "Introduced terminology" section to the "Bare issue summary template" default heading for new issues Fixed . Documentation for the section is available in https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-... →
Two special tags are available now, "Terminology addition" and "Terminology change". Documentation about the tags is included in https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-... →
- 🇬🇧United Kingdom longwave UK
Added to the default issue template and updated the documentation as per the examples in the IS. I also added an ID to the new h3 in the documentation so it can be linked if required.
The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.
- 🇫🇮Finland masipila
Apologies for replying to an old, already closed issue, but how do you actually construct a form array so that it only has the y-axis drag?
Comment #30 mentions that it would be configured using the
data-drag-orientation
attribute and it also highlights the need to have a change record which I was not able to find.Based on my tests it looks that the solution evolved to something else than what was mentioned in #30 (see example also below), but I can't figure out how to do this...
'#type' => 'table', '#attributes' => [ ...., 'data-drag-orientation' => 'drag-y', ],
Cheers,
Markus - 🇦🇺Australia jannakha Brisbane!
patch #466 applies to D10.3, but selected image styles are not visible in CKEditor 5 while adding/editing image:
Config works as expected:
Selected image styles are not available in CKEditor5:
- 🇬🇧United Kingdom AaronMcHale Edinburgh, Scotland
benjifisher → credited AaronMcHale → .
Automatically closed - issue fixed for 2 weeks with no activity.
- 🇳🇿New Zealand quietone New Zealand
This was tagged for a test in #14 for an error case. That change is no longer in the patch, so there is no need to test it, removing tag.
Since this is changing the UI it should have before/after screenshots, available from the issue summary.
- 🇪🇸Spain rcodina
@smustgrave Could you please check out current pipeline error? It doesn't make sense to me.
@alexpott All threads resolved.
- 🇳🇿New Zealand quietone New Zealand
If I read this correctly, in #8 @jhodgdon said there were no changes to be made on this page. Then, in #28 said that this could be changed to adjusting URLs here and elsewhere. But, URLs have been updated in 🐛 [META] Many documentation / handbook URLs redirect to D7 content Needs work and [##3248078]. So, as far as I can tell, there is nothing to do here. I am closing as outdated. If that is wrong add an explanatory comment and set the status to 'active'.