givewp
givewp copied to clipboard
Feature: GiveWP BlockTypes
Description
Introducing GiveWP BlockTypes!
The idea for GiveWP BlockTypes is to take generic block models and convert them into identifiable first-class BlockType models that have type-enabled properties and built-in casting.
Instead of this:
case "givewp/text":
return Text::make(
$block->hasAttribute('fieldName') ?
$block->getAttribute('fieldName') :
$block->getShortName() . '-' . $blockIndex
)->storeAsDonorMeta(
$block->hasAttribute('storeAsDonorMeta') ? $block->getAttribute('storeAsDonorMeta') : false
)->description($block->getAttribute('description'))
->defaultValue($block->getAttribute('defaultValue'))
We would have this:
case TextBlockType::name():
$textBlockType = new \Give\FormBuilder\BlockTypes\TextBlockType($block);
return Text::make(
$textBlockType->fieldName ?? $block->getShortName() . '-' . $blockIndex
)->storeAsDonorMeta($textBlockType->storeAsDonorMeta)
->description($textBlockType->description)
->defaultValue($textBlockType->defaultValue);
Thoughts:
- Would be nice to bridge the server block types and client blocks with a block.json spec. Maybe the
$properties
array actually gets generated by the block.json file one day 🤯 - Could explore more deep integration of these specific block types into our conversion process and/or programmatic block creation
- Integrating Block Types into the migration process would potentially be less tedious & more predictable.
- Sharing common attributes like
label
,fieldName
,conditionalLogic
, etc using traits. - Adding methods like
$blockType->toField()
would make the block to field conversion cleaner and more testable.
Affects
WIP
Visuals
N/A
Testing Instructions
This would need to be done programatically
Pre-review Checklist
- [ ] Acceptance criteria satisfied and marked in related issue
- [x] Relevant
@unreleased
tags included in DocBlocks - [x] Includes unit tests
- [ ] Reviewed by the designer (if follows a design)
- [ ] Self Review of code and UX completed
@kjohnson i've been gradually working on this during my Fridays. would love to get your thoughts on it.
This PR is stale because it has been open 45 days with no activity. Stale PRs will NOT be automatically closed.
@kjohnson anymore thoughts on this? If not, I would like to merge and start using it 🧙
anymore thoughts on this? If not, I would like to merge and start using it 🧙
No objections here. I gave it another look over and it looks good to me.
Having tests here is a big help!