Dakota/refactor collapsible block #377
Conversation
…rom svg file and make filterable
…SVG icons file in the block’s directory
| ><?php echo esc_html( "{$attributes['buttonOpenLabel']}" ); ?></button> | ||
| <?php endif; ?> | ||
| <div class="bu-block-collapsible-content js-bu-block-collapsible-content" id="<?php echo esc_attr( "{$attributes['id']}-panel" ); ?>"> | ||
| <?php echo $content; ?> |
There was a problem hiding this comment.
this is not escaped because the current version of the block isn't: https://github.com/bu-ist/bu-blocks/blob/develop/src/blocks/collapsible/index.php#L135
Do you think wp_kses_post() would be permissive enough to not have issues with outputting blocks and markup? The collapsible block allows a wide variety of nested blocks. I just don't want to break any existing blocks.
There was a problem hiding this comment.
Do we need to do any escaping or filtering here?
There was a problem hiding this comment.
Do you have any suggestions for what's left in this index.php file? Is this method of getting the SVG markup from the SVG file acceptable?
| checked={ autoID } | ||
| onChange={ () => setAttributes( { autoID: ! autoID } ) } | ||
| /> | ||
| <p> |
There was a problem hiding this comment.
Nice to have - is there any way to check that this is unique and flag it if not?
Additional notes to include:
- the id name is case sensitive
- the id name must contain at least one character and cannot start with a number
There was a problem hiding this comment.
The autoID here is just a toggle boolean attribute, defaulting to true that controls if the auto anchor id generation should be handled. I wrote this enhancement to the block a long while ago because the block was broken and putting duplicate anchor id's into production site and it was getting flagged in SiteImprove. It also was causing issues for the JS on the frontend and aria labels, etc.
You can see more about this in this file: https://github.com/bu-ist/bu-blocks/blob/dakota/refactor-collapsible-block/src/blocks/collapsible/generated-ids.js and the usage inside the Edit.js function.
I do like the idea of improving the ID name validation and I don't remember if it is currently checking for those two issues you point out. All of this is a bit of hack because WP core doesn't yet support a feature to handle this for attributes.
There is more background on this in the PR from a couple years ago where I introduced this auto generating ID's feature: #356
| // WordPress dependencies. | ||
| import { __ } from '@wordpress/i18n'; | ||
| import { | ||
| PanelBody, |
There was a problem hiding this comment.
PanelBody, ToggleControl, TextControl, RangeControl, Button, and ButtonGroup are not used and can be removed.
| import { | ||
| InnerBlocks, | ||
| BlockControls, | ||
| InspectorControls, |
There was a problem hiding this comment.
InspectorControls is not used and can be removed
| customMarginBottom, | ||
| marginBottom, | ||
| id, | ||
| buttonCloseLabel, |
There was a problem hiding this comment.
buttonCloseLabel is not used - not sure if that was missed or if it can be removed
There was a problem hiding this comment.
I think this got moved into the InspectorControls partial now, but let me double check it's still working
Later in future PR: