Remove references to 'props' outside of SDC - use 'inputs' instead

Created on 20 January 2025, 3 months ago

Overview

In πŸ“Œ Rename component source plugin configuration keys for clarity Active and πŸ“Œ Implement saving block settings forms Active we've been working on removing references to 'props' outside of the SDC source plugin.
This is to continue that work

Proposed resolution

User interface changes

πŸ“Œ Task
Status

Active

Version

0.0

Component

Miscellaneous

Created by

πŸ‡¦πŸ‡ΊAustralia larowlan πŸ‡¦πŸ‡ΊπŸ.au GMT+10

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @larowlan
  • πŸ‡¦πŸ‡ΊAustralia larowlan πŸ‡¦πŸ‡ΊπŸ.au GMT+10
  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    We'll need to update docs/components.md etc. as well 😊 Looking forward to making that all consistent and less confusing! πŸ€“πŸ˜…

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    … and also do generalize SDC's \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\SingleDirectoryComponent::getClientSideInfo() to not return field_data but input.

    This will allow BlockComponent::getClientSideInfo() to match that, and return the block plugin's default settings.

    That will allow instantiating an SDC component or a Block component to happen in identical ways, and hence allow removing the work-around that πŸ“Œ Implement saving block settings forms Active added.

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί
  • First commit to issue fork.
  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    Going to make a start on this.

  • Merge request !578First pass. β†’ (Merged) created by longwave
  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    Yay! :D

    And the remaining failures should be largely trivial to resolve, thanks to the detailed validation + test coverage we have:

    --- Expected
    +++ Actual
    @@ @@
     Array &0 [
    -    'component_tree' => 'The 'Drupal\Core\Block\TitleBlockPluginInterface' component interface must be absent.',
    -    'component_tree.props.block-invalid.' => Array &1 [
    -        0 => ''label' is a required key.',
    -        1 => ''label_display' is a required key.',
    +    'component_tree' => Array &1 [
    +        0 => 'The 'Drupal\Core\Block\TitleBlockPluginInterface' component interface must be absent.',
    +        1 => 'The array must contain an "inputs" key.',
         ],
     ]
    

    πŸ‘† Failures like that make it easy :)

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    99% of docs updates have now been done. Down to renaming, mostly :)

  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    I think I got everything that is not explicitly linked to SDCs or prop sources.

  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    Tempted to also renamed ComponentTreeStructure to just ComponentTree, to match ComponentInputs, given we have tree and inputs properties for them, and a tree is already a structure. ComponentTreeHydrated would then be better as HydratedComponentTree.

    Thoughts on this?

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    #12: Can you do those in an additional MR on the same issue (and do it on top of a squashed version of the current MR)? I'd like to keep the current MR focused on what it currently does, to keep it easy to review.

  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    All feedback has been addressed, this is ready for review again.

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    Docs

    I was working on the docs parts β€” I think it can be a lot clearer: by lifting all of the β€œshape matching” and β€œprop shape” and β€œprop source” stuff OUT of data-model.md AND components.md, and into a new shape-matching-into-field-types.md.

    That now means that those 2 pre-existing docs talk only about the general case, and leave the additional complexity for generating an input UX and storing those inputs for that new doc.

    It also means that redux-integrated-field-widgets.md can now refer to that more tightly scoped new doc instead of to the waaay bigger data-model.md.

    Stragglers

    I fixed a bunch of "component prop"stragglers (including renaming some of those to "SDC props") β€” if you search for it new, you'll find only 3 occurrences, and all 3 are for React component props, which is indeed an accurate term there.

    (This also helps make the Redux-integrated Field Widgets doc clearer, because I remember going back-and-forth with @bnjmnm about how confusing all the different "props" meanings were! πŸ€ͺ)

  • Pipeline finished with Skipped
    3 months ago
    #404618
  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί
  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    I figured rather than delaying the landing of this major clarification by awaiting review from @longwave, @larowlan and/or others, I'd just merge this. This is a major improvement compared to the state in HEAD!

    But please feel free to reopen this issue if you feel the docs are still not clear enough! πŸ™ I'm confident that what I did is an improvement, but I certainly do not think that my docs are perfect πŸ™ˆ

    IOW: I merged for the sake of enabling the majority of us working on XB to move on from this :)

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

Production build 0.71.5 2024