- Issue created by @amangrover90
- 🇺🇸United States mglaman WI, USA
We have some problems here:
- The code needs to move into the editor, how should we handle transliteration? Should we bubble the settings into the XB mount so it is accessible via configSlice?
- How would we access and use the transliteration library within React?
- I'm assuming this code needs to live within InputBehaviorsEntityForm for the path input, but can we know the value of the title from there? What is the best approach?
- We can't actually publish these entities due to the fact path fields are always invalid for new entities! See \Drupal\path\Plugin\Field\FieldWidget\PathWidget::validateFormElement().
foreach ($violations as $violation) { // Newly created entities do not have a system path yet, so we need to // disregard some violations. if (!$path_alias->getPath() && $violation->getPropertyPath() === 'path') { continue; } $form_state->setError($element['alias'], $violation->getMessage()); } }
I honestly don't know how to move forward here
- 🇮🇳India amangrover90
Blocked on https://www.drupal.org/project/experience_builder/issues/3504063 🐛 XB UI doesn't save the media/image or fields that involve Drupal javascript. Active
- 🇺🇸United States mglaman WI, USA
That's one reason we have this in the PageTrait for tests: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/path/...
protected static function assertSaveWithoutViolations(Page $page): void { // Path field is always invalid for new entities. // @see \Drupal\path\Plugin\Field\FieldWidget\PathWidget::validateFormElement().
From Core:
$element += [ '#element_validate' => [[static::class, 'validateFormElement']], ];
and in
validateFormElement
foreach ($violations as $violation) { // Newly created entities do not have a system path yet, so we need to // disregard some violations. if (!$path_alias->getPath() && $violation->getPropertyPath() === 'path') { continue; } $form_state->setError($element['alias'], $violation->getMessage()); } }
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Aha. It's the right level.
So the problem is that XB is not calling server-side widget logic. 📌 Support server side massage and validation of component prop form values Active is tackling a part of that, but it still won't call PHP
FieldWidget
server-side logic. See #3487284-6: [PP-3] Discover cases where is no 1:1 map between field, prop and widget values → , where that is being acknowledged.A more pragmatic short-term solution (and actually probably the better solution for Drupal core, too) is to:
- ignore at the
EntityConstraintViolationList
level this validation error - for any
\Drupal\path\Plugin\Field\FieldType\PathItem
field instance on the validated content entity - if the content entity is new (
::isNew() === TRUE
- ignore at the
- Status changed to Postponed
16 days ago 11:42am 17 May 2025 - First commit to issue fork.
- 🇫🇮Finland lauriii Finland
Converted the JavaScript code to a React component and implemented it. Updated the issue summary to match the implementation.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
That looks super promising, and a much nicer UX! 🤩
I just responded in detail at #3523029-22: Content entity form: URL alias without leading slash should be prevented by client-side validation → , but per your comment at #3523029-21: Content entity form: URL alias without leading slash should be prevented by client-side validation → , we probably want to close that in favor of this? Especially given you've pretty much … implemented it 😅
Review
Questions:
- it's restricted to the
Page
content entity type. But e.g. article nodes' path alias should have this same UX? - And what about SDC props that use
type: string, format: uri-reference
? (The auto-generating part wouldn't make sense there, but the better client-side validation would.) - If the answer to the above is that we shouldn't make it work for component instances, then … why don't we just create a new widget and at-runtime override the entity form display to not use
\Drupal\path\Plugin\Field\FieldWidget\PathWidget
?
- it's restricted to the
- First commit to issue fork.
- 🇺🇸United States bnjmnm Ann Arbor, MI
- The MR capabilities are a variantion Pathauto does, and as a result conflicts. It that module is enabled, it will ultimately override whatever is created by this, but confusingly ly, even if the "Generate automatic URL alias" checkbox is filled, the disabled URL alias field will populated with the autogenerated string. (note that this checkbox does not yet work for everyone - I tested this with a branch from issue #3481719 which gets the State API working). I believe pathauto might be bundled with some systems that use experience builder, so we'd need to account for this
- Re #15.2 "And what about SDC props that use type: string, format: uri-reference" is this a concern here? I believe those formats use the widget for link fields, not path fields.
- 🇫🇮Finland lauriii Finland
There's some more work needed to get pathauto to fully work with XB: 🐛 XB pages doesn't respect Pathauto widget Active . We need to do that issue regardless of what we do here.
I think we should do the following for the pathauto integration in the other issue:
- Make pathauto not break the path field when there's no pattern configured for the entity type
- Make sure that the solutions do not conflict, i.e. this should be only loaded when pathauto hasn't been enabled
I'd be more than happy to tackle 2. but I think we need to fix 1. more urgently first.
+1 for
type: string, format: uri-reference
not being relevant here. - 🇺🇸United States bnjmnm Ann Arbor, MI
I have some MR feedback. I think we can reduce the total renders of the Path widget component by using refs for some items managed by state, but I don't have time ATM to confirm that yet. Will come back to that shortly.
- 🇺🇸United States bnjmnm Ann Arbor, MI
Approved the MR, still needs approval from /
src/
and/src/Entity/
Also noticed #15.2 was already discussed but probably worth conclusively answering 1 and 3 even if the answer is sort of implied already just to be sure we're properly acknowledging every review point
- 🇫🇮Finland lauriii Finland
it's restricted to the Page content entity type. But e.g. article nodes' path alias should have this same UX?
This is currently restricted to Pages (since that's all that XB supports). We could expand this to Nodes in future once we work on those.
If the answer to the above is that we shouldn't make it work for component instances, then … why don't we just create a new widget and at-runtime override the entity form display to not use \Drupal\path\Plugin\Field\FieldWidget\PathWidget?
I don't think we would make this work with component instances since this is specific to the path field type which is tightly coupled with aliases. Not creating a new widget makes adding Pathauto support simpler most likely because Pathauto is extending the default widget. If that's not the case, it seems this would be really easy to refactor to a widget if we want to do that as a follow-up to 🐛 XB pages doesn't respect Pathauto widget Active .
-
lauriii →
committed 21525826 on 0.x authored by
amangrover90 →
Issue #3499960 by lauriii, amangrover90, bnjmnm, mglaman, wim leers:...
-
lauriii →
committed 21525826 on 0.x authored by
amangrover90 →
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺