-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat (tabs): added option to make height equal throughout the tab's contents #2926
Conversation
🤖 Pull request artifacts
|
Size Change: +1.92 kB (0%) Total Size: 1.98 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments:
-
When you already have a tabs block and you update, in the backend, your tab content will resize smaller when changing tabs (it should not)
-
When you already have a tabs block and you update, the tabs became not equal height. The option should be pre-enabled for existing tabs. (although if I hit preview, the frontend still showed equal height)
-
I couldn't find the option to change the height, please move it to the main tabs block and not the tab content block.
-
Change the option label to "Equal tab height"
@bfintal i temporarily changed the stackable version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see my comment about the >
in the selector and about migration.
Also, kindly clean up the PR, there is an imported i18n
that's no longer used and a stray console.log
src/block/tabs/style.js
Outdated
@@ -43,6 +43,18 @@ const Styles = props => { | |||
format="%spx" | |||
responsive="all" | |||
/> | |||
<BlockCss | |||
{ ...propsToPass } | |||
selector=".%s .stk-block-tab-content > .stk-block-content > .stk-block-column[hidden]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We cannot have >
signs in the selector for the save render. WP has a bug that it will throw an error in multisite.
Please change the implementation so we do not have >
in the selector. Note that this is okay for edit
src/block/tabs/deprecated.js
Outdated
return { | ||
...attributes, | ||
equalTabHeight: true, | ||
} | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, you will need to include the things in the older migrate method for 3.11.9:
let newAttributes = { ...attributes }
newAttributes = deprecateContainerBackgroundColorOpacity.migrate( newAttributes )
newAttributes = deprecateBlockBackgroundColorOpacity.migrate( newAttributes )
return newAttributes
This is because deprecation only stops when it encounters the first satisfied deprecation, so all users with an old tabs block will never get migration to the new combined opacity and color.
fixes #2844