Problem/Motivation

With #3514072: SDC slots can set expectations and cardinality , Core will introduce 3 new optional keywords in each SDC slot definition:

  • expected with a list of values: SDC plugin IDs (if they have a colon inside) or tags/groups (if no colon inside)
  • minItems and maxItems with a strictly positive integer value: Enforce a lower/maximum limit on the number of children

SDC and the Render API will only suggest, not enforce. So, both UI Patterns 2 (in the slot part of the component form) and Display Builder will need to find a way to leverage those properties.

ContextualForm is out of scope

For Display Builder, it will be about builder & layers panels only. Everything happening in the ContextualForm panel, even involving slots, will be manged by UI Patterns 2. An issue will be created there.

The content overrides case

For content overrides, we need to also support the maxItems mechanism. Because the cardinality of the UI Patterns Soure field (" Allowed number of values " from the Field API) is acting like the maxItems keyword proposed in the Core issue, on the root dropzone (which is a slot of a "virtual" component).

Proposal

We don't need to wait the Core issue to be merged to start. The consensus has been reached and Core will not do much anyway.

General:

  • Let's ignore minItems for now.
  • SDC and the Render API will only suggest, not enforce. So, it would be great to have a config in the Profile, or in the island(s), so we can apply strict behaviour to only some user roles
  • If a logic would be better in UI Patterns level, go for it, to avoid duplicating logic later when UIP2 will need the same mechanism
  • The challenge will be the UI. What do we do? Disactivate dropzones according to the component

Specific to maxItems:

  • For builder & layers panels
  • Content override: Limit the number of sources we can put in the root dropzone by checking the cardinality of the content field
  • The restriction works for all sources, because if a source can have many renderables but many sources can't generate a single renderable
  • because if a source can have many renderables, we may overpass the limit without knowing it but it is OK, this is the sitaution we can accept

expected will have its own ticket: #3617065: Expected components constraints for slots

Other follow-ups

Issue fork display_builder-3544026

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

pdureau created an issue. See original summary.

pdureau’s picture

pdureau commented
Title: Cardinality constraint for slots » Cardinality & suggested components constraints for slots
Issue summary: View changes
pdureau’s picture

pdureau commented
Issue summary: View changes
pdureau’s picture

pdureau commented
Issue tags: -display_builder-1.0.0-beta +display_builder-1.0.0-beta2
pdureau’s picture

pdureau commented
Assigned: Unassigned » just_like_good_vibes

Mikael is taking the related UIP2 issue #3551587: Slots restrictions according to suggestions and cardinality so it is the best candidate for this task

pdureau’s picture

pdureau commented
Issue tags: -display_builder-1.0.0-beta2 +display_builder-1.0.0-beta4

Careful, there are some changes at Druapl Core level: #3514072: SDC slots can set expectations and cardinality

Please follow closely what is happening there.

svendecabooter’s picture

svendecabooter
he/him
Primary language Dutch
Location Gent
commented
Related issues: +#3544026: Cardinality constraints for slots
svendecabooter’s picture

svendecabooter
he/him
Primary language Dutch
Location Gent
commented
Related issues: -#3544026: Cardinality constraints for slots +#3514072: SDC slots can set expectations and cardinality
pdureau’s picture

pdureau commented
Related issues: +#3551232: Add a lock system

This change is also needed for #3551232: Add a lock system

mogtofu33’s picture

mogtofu33 commented
Component: display_builder_entity_view » Code
pdureau’s picture

pdureau commented
Issue tags: -display_builder-1.0.0-beta4 +display_builder-1.0.0-beta
pdureau’s picture

pdureau commented

#3514072: SDC slots can set expectations and cardinality has been merged to Core main branch tooday

pdureau’s picture

pdureau commented
Assigned: just_like_good_vibes » pdureau
Issue summary: View changes

I will give a try

pdureau opened merge request !274

pdureau’s picture

pdureau commented
Category: Task » Feature request

This is a feature request

pdureau’s picture

pdureau commented
Status: Active » Needs work

It has started by the way ;)

pdureau’s picture

pdureau commented

Some parts of the scope has been moved to #3617065: Expected components constraints for slots

So, let's focus on maxItems:

For builder For scaffold In Controller & Instance
In buildable root
  • ✅ implementation
  • 🎯 kernel tests
  • 🎯 playwright tests
  • ✅ implementation
  • 🎯 kernel tests
  • 🎯 playwright tests

>

  • ✅ implementation
  • ✅ kernel tests
  • playwright tests
In SDC slot
  • ✅ implementation
  • ✅ kernel tests
  • 🎯 playwright tests
  • ✅ implementation
  • ✅ kernel tests
  • 🎯 playwright tests
  • ✅ implementation
  • ✅ kernel tests
  • playwright tests

Also:

  • ❌ the JS part is not finished, when we block the addition of a source in a locked slot, the HTMX request is sent anyway
  • ⚠️ how do we give visual clue to the user than a slot is locked/full?

We now have at least 4 other tickets waiting for this feature:

pdureau’s picture

pdureau commented
Title: Cardinality & suggested components constraints for slots » Cardinality components constraints for slots
Issue summary: View changes
pdureau’s picture

pdureau commented
Related issues: +#3617065: Expected components constraints for slots
pdureau’s picture

pdureau commented
Title: Cardinality components constraints for slots » Cardinality constraints for slots
pdureau’s picture

pdureau commented
Assigned: pdureau » mogtofu33
Status: Needs work » Needs review

Current status:

For builder For scaffold In Controller & Instance
In buildable root
  • ✅ implementation
  • ✅ kernel tests
  • 🎯 playwright tests
  • ✅ implementation
  • ✅ kernel tests
  • 🎯 playwright tests
  • ✅ implementation
  • ✅ kernel tests
  • playwright tests
In SDC slot
  • ✅ implementation
  • ✅ kernel tests
  • 🎯 playwright tests
  • ✅ implementation
  • ✅ kernel tests
  • 🎯 playwright tests
  • ✅ implementation
  • ✅ kernel tests
  • playwright tests

How do this work?

  • Tree root, browser side:
    1. ViewPanelBase is adding data-max-items attribute to dropzone
    2. According to DisplayBuildableInterface::getRootCardinality()
    3. dropzone.js is preventing the drop.
  • Tree root, server side:
    1. ApiContoller::attachToRoot() is listening OutOfRangeException
    2. Triggered by Instance::attachToRoot() and Instance::moveToRoot()
    3. When the root is full according to the new DisplayBuildableInterface::getRootCardinality()
  • SDC slots, browser side:
    1. RealRenderTrait is adding data-max-items attribute to dropzone
    2. According to SourceWithSlotsInterface::getSlotCardinality()
    3. dropzone.js is preventing the drop
  • SDC slots, server side:
    1. ApiContoller::attachToSlot() is listening OutOfRangeException
    2. Triggered by Instance::attachToSlot() and Instance::moveToslot()
    3. When the slot is full according to the new SourceWithSlotsInterface::getSlotCardinality()

Topics to discuss during review:

  • the architecture & implementation: has the added logic been put at the right place? For example, some stuff in Instance entity could be moved to SourceTree, I was hesitating.
  • UX: how do we give visual clue to the user than a slot is locked/full? Do we address this in a follow-up?
  • the missing playwright tests: i may need help
  • documentation update: I can do it once we are both OK with the change

For information, the other tickets waiting for this mechanism:

mogtofu33’s picture

mogtofu33 commented
Assigned: mogtofu33 » pdureau
Status: Needs review » Needs work

Pushed 4 fixes in individuals commit for review, so they can be discussed/reverted if problem.

  • Architecture: isSlotFull() instantiates a throwaway source plugin separate from what SlotSourceProxy already resolves. Real argument for moving to SourceTree, but a refactor, not required here.
  • UX: Fine as follow-up, not a blocker.
  • Playwright: will try to add the missing scenarios if everything else is ok
  • Docs: I think we have a whole pass required from last beta, but I would like more an onboarding user doc first, then the technical
pdureau’s picture

pdureau commented

Thanks for the quick review and the 4 welcomed changes 👍

Architecture: isSlotFull() instantiates a throwaway source plugin separate from what SlotSourceProxy already resolves. Real argument for moving to SourceTree, but a refactor, not required here.

I will try this a bit.

UX: Fine as follow-up, not a blocker.

OK

Playwright: will try to add the missing scenarios if everything else is ok

Thanks a lot. I will assign the ticket to you once ready.

Docs: I think we have a whole pass required from last beta, but I would like more an onboarding user doc first, then the technical

OK, so I will just add a few information, and we will do the whole pass later.

pdureau’s picture

pdureau commented

Architecture: isSlotFull() instantiates a throwaway source plugin separate from what SlotSourceProxy already resolves. Real argument for moving to SourceTree, but a refactor, not required here.

Done.

pdureau’s picture

pdureau commented
Assigned: pdureau » mogtofu33
Status: Needs work » Needs review

Documentation changed, not much because of the expected whole pass, but at least cardinality is mentioned:

  • in docs/entity-displays-overrides.md
  • in the new docs/sdc.md

I have also added mentions of:

  • page url context in docs/internals.md
  • Instances & State islands, and grouping by regions, in docs/islands.md
mogtofu33’s picture

mogtofu33 commented
Assigned: mogtofu33 » Unassigned
Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • mogtofu33 committed 78f769c6 on 1.0.x authored by pdureau 
    feat: #3544026 Cardinality constraints for slots

By: pdureau
By:...

Status: Fixed » Closed (fixed)

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