-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Adds tabs block behind block experiments flag #63689
Conversation
Size Change: +12.9 kB (+0.73%) Total Size: 1.78 MB
ℹ️ View Unchanged
|
There is an issue open for this block at #34079. I also worked on the same block last week, and I have a working prototype, but it's outside the Gutenberg repository in case you would like to resue any parts of the code: a8cteam51/special-projects-blocks-monorepo#12. As noted in a8cteam51/special-projects-blocks-monorepo#12 (comment), I'm not going to work on this block in the next weeks actively. |
49f8565
to
d9dbd90
Compare
4c4d53f
to
a5b02b9
Compare
@gziolo @luisherranz I'm trying to get the interactivity API working with this block and could use some help! Here's the commit that adds it: a5b02b9 I'm seeing this JS console error in the editor (which is strange, because I didn't think
And I'm seeing this error on the front end (is seems like a script dependency isn't getting set correctly):
|
If I am not mistaken, currently in Gutenberg, we are manually enqueuing the frontend files. So try adding a render callback where you include this code (or something similar): $suffix = wp_scripts_get_suffix();
if ( defined( 'IS_GUTENBERG_PLUGIN' ) && IS_GUTENBERG_PLUGIN ) {
$module_url = gutenberg_url( '/build/interactivity/tab.min.js' );
}
wp_register_script_module(
'@wordpress/block-library/tab',
isset( $module_url ) ? $module_url : includes_url( "blocks/tab/view{$suffix}.js" ),
array( '@wordpress/interactivity' ),
defined( 'GUTENBERG_VERSION' ) ? GUTENBERG_VERSION : get_bloginfo( 'version' )
);
wp_enqueue_script_module( '@wordpress/block-library/tab' ); And add the view.js file to this list (then restart Webpack). Let me know if that works because there might be other steps that I don't remember right now. |
By the way, if you are testing, that's fine, but the final version should not include any directives in the markup serialized to the database. They should be injected using the HTML tag processor in the render callback. That way, the frontend implementation can be changed or improved without deprecations. |
90af81e
to
c6e1846
Compare
This allows the block to display content in non-browser contexts like email.
… RichText tab titles
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
This seems to be working pretty well now and is ready for review. |
* | ||
* @param {HTMLElement} ref The block wrapper element. | ||
*/ | ||
function initTabs( ref ) { |
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.
What's the reason for doing all these DOM manipulations here manually instead of using directives for it?
This way they won't get server rendered and there will be a flash of unsettled content as the page loads 🤔
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.
Exactly. Please use the correct directives for all the DOM manipulations.
Here's a good example of a block whose functionality is similar to this one. Take a special look at the frontend file: it only modifies the local context. All the DOM manipulations are done through directives that are linked to that local context. Also, notice how the local context is initialized on the server to ensure everything is ready before the JS downloads and executes.
This is not published yet, but maybe this guide, which is intended to explain this declarative approach, can help (I'd also appreciate any feedback regarding the guide itself 🙂).
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.
@fabiankaegy and @luisherranz Thank you for the feedback!
Right now, by default, the tabs block is a list of on page links, and then progressively enhanced into tabs when JS is loaded.
How I can add the attributes on the server and still progressively enhance the block depending on if JS is loaded or not? Is there a way to use the interactivity API to add the role and aria attributes conditionally?
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.
Is there a way to use the interactivity API to add the role and aria attributes conditionally?
Perhaps the guides that will be published here can help you understand how the Interactivity API works?
This is not published yet, but maybe this guide, which is intended to explain this declarative approach, can help (I'd also appreciate any feedback regarding the guide itself 🙂).
I know I'm repeating myself, but I think the accordion block example is really helpful for understanding how to program something like this, since the implementation of an accordion is quite similar to the implementation of tabs.
Here's a good example of a block whose functionality is similar to this one. Take a special look at the frontend file: it only modifies the local context. All the DOM manipulations are done through directives that are linked to that local context. Also, notice how the local context is initialized on the server to ensure everything is ready before the JS downloads and executes.
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.
This is not published yet, but maybe this guide, which is intended to explain this declarative approach, can help (I'd also appreciate any feedback regarding the guide itself 🙂).
The guide was published here.
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.
@fabiankaegy, @luisherranz, @DAreRodz In cc304b5, I've removed the initTabs
callback in favor of server-side directives and state getters. This is less code overall, so thank you for the guidance.
One thing clarify is that the tabs front-end here has been built as progressively enhanced markup, as suggested by the likes of https://design-system.service.gov.uk/components/tabs/ and https://inclusive-components.design/tabbed-interfaces/. Without JS, it's simply a list of on page links:
This means that I'm not adding attributes directly with the interactivity API ($p->set_attribute( 'role', 'tab' );
), but instead using the wp-bind
directive ($p->set_attribute( 'data-wp-bind--role', 'state.roleAttribute' );
) with a getter in the view.js
state that returns the correct role based on the element's class name. The role
, tabindex
, and aria-
attributes are semantically incorrect if JS is not loaded on the page, as there is no tabbed interface, so I don't think they should be present directly in the server-side markup.
For attributes with dynamic values this works well. It would also be nice to have a way to do this more simply with static values. For example, $p->set_attribute( 'data-wp-bind--role', 'tablist
)` doesn't work right now, but this would simplify the code even more, as the role property never changes, it just needs to be added when JS is initialized.
I love that this is coming to core.🎉 Great work
Screen.Recording.2024-08-05.at.10.44.04.mov |
Hey @creativecoder 👋 I've been working on a version of this block for client projects for a while now and the solution we came up with for having the tab button / label RichText actually live inside the tab item block so that it properly gets selected when you interact with it was using the const { Fill, Slot } = createSlotFill('TabHeader');
export const TabHeader = ({ children, tabsClientId, orientation }) => {
return (
<Fill orientation={orientation} name={`TabHeader-${tabsClientId}`}>
{children}
</Fill>
);
};
export const TabHeaderSlot = ({ tabsClientId, orientation }) => {
return (
<Slot
orientation={orientation}
name={`TabHeader-${tabsClientId}`}
bubblesVirtually
as="ul"
className="tab-list wp-block-tabs__tab-list"
/>
);
}; With that slot setup I can then use the following inside the edit.js of the Tab Item block: <TabHeader tabsClientId={parentBlockClientId} orientation={orientation}>
<li
className={clsx('tab-item', 'wp-block-tabs__tab-item', {
'is-active': isSelectedTab,
})}
>
<RichText
tagName="span"
withoutInteractiveFormatting
value={label}
onChange={(value) => setAttributes({ label: value })}
/>
</li>
</TabHeader>
{isSelectedTab && (
<>
<div className="tab-control wp-block-tabs__tab-control">
<TabHeaderSlot
tabsClientId={parentBlockClientId}
orientation={orientation}
/>
</div>
<div className="tab-group wp-block-tabs__tab-group">
<div {...innerBlocksProps} />
</div>
</>
)} And in the parent Tabs block I have this: <TabHeader tabsClientId={clientId} orientation={orientation}>
{isSelected && (
<li
className="tab-item wp-block-tabs__tab-item tab-item--inserter"
>
<InnerBlocks.ButtonBlockAppender
icon="plus"
onClick={appendTabItem}
/>
</li>
)}
</TabHeader> CleanShot.2024-08-05.at.11.06.00.mp4 |
This is looking great! But as work continues, I want to highlight some extensibility "opportunities" that might be worth considering since this will be a new block.
|
* | ||
* @param {HTMLElement} ref The block wrapper element. | ||
*/ | ||
function initTabs( ref ) { |
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.
Exactly. Please use the correct directives for all the DOM manipulations.
Here's a good example of a block whose functionality is similar to this one. Take a special look at the frontend file: it only modifies the local context. All the DOM manipulations are done through directives that are linked to that local context. Also, notice how the local context is initialized on the server to ensure everything is ready before the JS downloads and executes.
This is not published yet, but maybe this guide, which is intended to explain this declarative approach, can help (I'd also appreciate any feedback regarding the guide itself 🙂).
Noting here that I'm going to be AFK for a couple of weeks--I plan to return to working on this as soon as I get back! |
Just noting something that was mentioned in the recent Hallway Hangout: Have we considered introducing the idea of block states for the Tabs block (tab open/closed)? |
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.
Thank you for working through this block.
I've left comments to the best of my understanding, but please let me know if I've misunderstood something 👍
* | ||
* @return {string} The generated slug with HTML stripped out. | ||
*/ | ||
function slugFromLabel( label, tabIndex ) { |
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.
I think with this approach, if the same tab label exists in multiple tab blocks, we will get duplicate tab ids.
One approach would be to generate a unique ID using something like wp_unique_id()
function when rendering on the server side, but I don't know how to share that ID between Tabs and the Tab block 🤔
const { anchor, isActive, label, slug, tabIndex } = attributes; | ||
// Use a custom anchor, if set. Otherwise fall back to the slug generated from the label text. | ||
const tabPanelId = anchor || slug; | ||
const tabLabelId = tabPanelId + '--tab'; |
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.
there is a possibility of duplicate IDs. Can't we use the clientId
or the useInstanceId()
hook?
const blockProps = useBlockProps.save(); | ||
const innerBlocksProps = useInnerBlocksProps.save( blockProps ); | ||
|
||
return <section { ...innerBlocksProps } id={ tabPanelId } />; |
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.
Why a section
element?
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.
Using <section>
elements for tab panels is recommended here: https://inclusive-components.design/tabbed-interfaces/
> *:first-child { | ||
margin-top: 0; | ||
} |
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.
> *:first-child { | |
margin-top: 0; | |
} | |
> *:first-child { | |
margin-top: 0; | |
} | |
> *:last-child { | |
margin-bottom: 0; | |
} |
[ 'core/tab', { label: 'Tab 1' } ], | ||
[ 'core/tab', { label: 'Tab 2' } ], |
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.
[ 'core/tab', { label: 'Tab 1' } ], | |
[ 'core/tab', { label: 'Tab 2' } ], | |
[ 'core/tab', { label: __( 'Tab 1' ) } ], | |
[ 'core/tab', { label: __( 'Tab 2' ) } ], |
}, | ||
{ | ||
__experimentalCaptureToolbars: true, | ||
clientId, |
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.
If I understand correctly, clientId
is not defined as an option for the useInnerBlocksProps
hook.
<h3 className="wp-block-tabs__title"> | ||
{ /* translators: Title for a list of content sections linked below. */ } | ||
{ __( 'Contents' ) } | ||
</h3> |
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.
The title appears to be hidden with CSS, so why is it needed? Also, hard-coded heading levels can lead to incorrect heading structures depending on the structure of the content.
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.
This was also recommended by https://inclusive-components.design/tabbed-interfaces/. Without front-end JS, the tabs block a list of on page links; this heading makes sure the list has a title, but is hidden when JS is loaded and the list is enhanced into a tabbed interface.
Also, hard-coded heading levels can lead to incorrect heading structures depending on the structure of the content.
That's a good point. Alternatively role="heading"
could be used here, but I'm not sure that's any better--https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/heading_role indicates it defaults to heading level 2.
</h3> | ||
|
||
<ul className="wp-block-tabs__list"> | ||
{ innerTabs.map( ( tab, index ) => { |
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.
@creativecoder just a heads up that since #65064, you'll need to do something like this to load the view script for the tabs block: 84112e3 |
Closing this, as I won't be able to work on it further. |
What?
Adds an experimental tabs block. To enable it, turn on Experimental blocks from Gutenberg > Experiments in wp-admin.
Why?
To enable tabbed content for sites and themes. See #34079. Part of #63501.
How?
Includes 2 blocks:
This first version is behind the block experiments so that we can iterate as needed without writing block deprecations.
Testing Instructions
/wp-admin/admin.php?page=gutenberg-experiments
and turn on Experimental blocksOther features to test
Testing Instructions for Keyboard
In the front-end:
Screenshots or screencast
Editor
Screen.Recording.2024-08-04.at.22.30.22.mov
Front-end
Screen.Recording.2024-08-04.at.22.33.28.mov
Further work
This initial version is intentionally minimal to try and reduce the amount of code to review. Items for further work in future PRs include
tabindex="0"
to the active tab panel only if it does not contain content where the first element in the panel is focusable (see ARIA example)