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.

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:

Comments

pdureau created an issue. See original summary.

pdureau’s picture

pdureau’s picture

Issue summary: View changes

According 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.

pdureau’s picture

Issue summary: View changes
pdureau’s picture

Assigned: Unassigned » pdureau

I will try to do it.

pdureau’s picture

Status: Active » Needs work
pdureau’s picture

Status: Needs work » Needs review

Done.

Kept on my side because I will do extra checks before sending it to review.

pdureau’s picture

Assigned: pdureau » mogtofu33

OK for review

mogtofu33’s picture

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

When 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.

pdureau’s picture

Thanks for the feedback. I will work on that

pdureau’s picture

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

Config schema has been completed

mogtofu33’s picture

Assigned: mogtofu33 » Unassigned
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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