Problem/Motivation
Display builder profile config entity have both island_settings & island_configuration:
island_settings:
view:
builder:
enable: true
weight: '-10'
options: main
layers:
enable: true
weight: '-9'
options: main
preview:
enable: true
weight: '-8'
options: main
button:
history:
enable: true
weight: '-8'
state:
enable: true
weight: '-7'
...
island_configuration:
history:
display_clear_button: 1
This is confusing, not enough defined (type: ignore), and may introduce bugs because:
island_settings follows to closely the form structure
- we need to maintain 2 lists with the same keys
Proposed resolution
Can we have a merged flat structure managed by something like Drupal\Component\Plugin\LazyPluginCollection
Proposal:
islands:
builder:
enable: true
weight: -10
region: main
layers:
enable: true
weight: -9
region: main
preview:
enable: true
weight: -8
options: main
history:
enable: true
weight: -8
settings:
display_clear_button: 1
state:
enable: true
weight: -7
...
This is also the opportunity to allow configuration of islands before first config save and to remove:
Island configuration will be available only after saving this form.
Comments
Comment #2
pdureau commentedComment #3
pdureau commentedAccording to Christian, the reason for 2 properties is to avoid to override values because they are not written by the same controller.
According to Jean it may be possible to merge them now but we need to check.
Comment #4
pdureau commentedComment #5
pdureau commentedI will try to do it.
Comment #7
pdureau commentedWaiting for #3539519: Split DisplayBuilderForm::form() to ease maintenance to be merged
Comment #8
pdureau commentedDone.
Kept on my side because I will do extra checks before sending it to review.
Comment #9
pdureau commentedOK for review
Comment #10
mogtofu33 commentedWhen changing any configuration we have a schema error, because each island configuration will be added and is not defined in the schema, for example configure the history on profile default to enable clear button, save.
Error schema:
islands.history.display_clear_button Undefined undefined No 1 missing schema 'display_clear_button' is not a supported key.
Previously it was ignored, as we already have keys defined (enable / weight / region), I am not sure how we can add unknown keys to match any configuration values.
Comment #11
pdureau commentedThanks for the feedback. I will work on that
Comment #12
pdureau commentedConfig schema has been completed
Comment #14
mogtofu33 commented