fix (icon label): change icon gap #3209
Conversation
🤖 Pull request artifacts
|
There was a problem hiding this comment.
While this should work, I want to suggest whether we can try a different (cleaner) approach.
Approach:
- We create a new attribute
iconGap2that holds the new value foriconGap, moving forwardiconGapwill no longer be used, but it will still be kept in the attribute definition - In PHP, in
render_blockif we encounter the block that has aniconGapattribute, but doesn't have aniconGap2, we:- Calculate for the new icon gap value, then replace the old CSS with the updated CSS
- Add a special class to say that this block should use the old css
flex: 0 0 64px
- In the editor JS, we have this deprecation entry:
isEligiblereturns true if the block contains a value foriconGap, the migrate the value fromiconGaptoiconGap2
Pros of approach:
- No need to create a new
versionattribute - No need for a new class name
src/block/icon-label/index.php
Outdated
| $attributes = $block[ 'attrs' ]; | ||
|
|
||
| // check if the block uses the old icon gap attribute | ||
| $use_old_styles = ( isset( $attributes[ 'iconGap' ] ) || isset( $block[ 'innerBlocks' ][0]['attrs']['iconSize'] ) ) && ! isset( $attributes[ 'iconGap2' ] ); |
There was a problem hiding this comment.
Check first for the presence of the elements in $block['innerBlocks'] to prevent fatal errors.
src/block/icon-label/index.php
Outdated
| @@ -0,0 +1,25 @@ | |||
| <?php | |||
|
|
|||
| // Exit if accessed directly. | |||
There was a problem hiding this comment.
can we rename this file to deprecated.php or something so we can denote that it's for deprecation purposes?
| @@ -37,6 +37,11 @@ export const attributes = ( version = VERSION ) => { | |||
| stkResponsive: true, | |||
| default: '', | |||
| }, | |||
| iconGap2: { | |||
There was a problem hiding this comment.
Add a comment on iconGap that it's deprecated but kept here for migration purposes
src/block/icon-label/style.js
Outdated
| @@ -26,6 +26,7 @@ const Styles = props => { | |||
|
|
|||
| return ( | |||
| <> | |||
| { /* For the old icon gap */ } | |||
There was a problem hiding this comment.
I believe you should be able to remove the BlockCss entry for the old iconGap here and the block should not error out.
src/block/icon-label/style.scss
Outdated
| // Default width of the icon width. | ||
| .stk-block-icon { | ||
| flex: 0 0 64px; | ||
| // For old icon gap |
There was a problem hiding this comment.
can you move this css to the bottom (isolating it from the rest of the "current" css code) and then label it as "deprecated"
src/block/icon-label/editor.scss
Outdated
| flex: 0 0 0; | ||
| } | ||
| // Icon label can collapse in mobile in the editor. | ||
| .stk-preview-device-mobile .stk-block-icon-label .block-editor-block-list__layout [data-type="stackable/icon"] { | ||
| flex: 0 0 64px; | ||
| flex: 0 0 0; |
There was a problem hiding this comment.
Does it work if we remove the flex definition completely since this is for the old 0 0 64px?
There was a problem hiding this comment.
it still needs the flex, but i changed it to flex: 0 instead.
|
@mxkae Issue with Icon Gap has now been fixed. One remaining issue:
Screen.Recording.2024-06-24.at.3.mp4 |
fixes #3126