Skip to content
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

Editor package: invalid DOM nesting in PostPublishPanelToggle component #9033

Closed
vindl opened this issue Aug 15, 2018 · 4 comments · Fixed by #17342
Closed

Editor package: invalid DOM nesting in PostPublishPanelToggle component #9033

vindl opened this issue Aug 15, 2018 · 4 comments · Fixed by #17342
Assignees
Labels
[Feature] NUX Anything that impacts the new user experience [Package] Editor /packages/editor [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Milestone

Comments

@vindl
Copy link
Member

vindl commented Aug 15, 2018

Describe the bug

Using the PostPublishPanelToggle component from @wordpress/editor package produces the following console warning:

Warning: validateDOMNesting(...): <button> cannot appear as a descendant of <button>.

This is caused by the fact that PostPublishPanelToggle nests a DotTip inside of a button:

<Button
className="editor-post-publish-panel__toggle"
isPrimary
onClick={ onToggle }
aria-expanded={ isOpen }
disabled={ ! isButtonEnabled }
isBusy={ isSaving && isPublished }
>
{ isBeingScheduled ? __( 'Schedule…' ) : __( 'Publish…' ) }
<DotTip id="core/editor.publish">
{ __( 'Finished writing? That’s great, let’s get this published right now. Just click ‘Publish’ and you’re good to go.' ) }
</DotTip>
</Button>

and DotTip in turn renders a button too (or two if IconButton renders it):

<p>
<Button isLink onClick={ onDismiss }>
{ hasNextTip ? __( 'See next tip' ) : __( 'Got it' ) }
</Button>
</p>
<IconButton
className="nux-dot-tip__disable"
icon="no-alt"
label={ __( 'Disable tips' ) }
onClick={ onDisable }
/>

To Reproduce

import { PostPublishPanelToggle } from '@wordpress/editor';

...

render() {
	return <PostPublishPanelToggle />;
}

Related issue

A similar issue exist in edit-post's header component, where DotTip is nested inside of IconButton:

https://github.com/WordPress/gutenberg/blob/master/edit-post/components/header/index.js#L57-L68

@vindl vindl added [Type] Bug An existing feature does not function as intended [Package] Editor /packages/editor labels Aug 15, 2018
@vindl
Copy link
Member Author

vindl commented Aug 15, 2018

cc @youknowriad @gziolo @aduth

@gziolo gziolo added the [Feature] NUX Anything that impacts the new user experience label Aug 15, 2018
@gziolo
Copy link
Member

gziolo commented Aug 15, 2018

/cc @noisysocks

@aduth
Copy link
Member

aduth commented Aug 15, 2018

Previously: #6631 (comment)

@noisysocks noisysocks self-assigned this Aug 20, 2018
@noisysocks noisysocks added this to the WordPress 5.0.x Follow Ups milestone Nov 26, 2018
@noisysocks noisysocks removed their assignment Jul 4, 2019
@pento
Copy link
Member

pento commented Sep 5, 2019

This issue (or, more broadly, the problem of using a <DotTip> inside a <Button>) is a blocker for making the dev environment more reliable.

It can currently be reproduced by running e2e tests with SCRIPT_DEBUG set to true. If you're not sure if your local environment is configured to reproduce it, you can rebuild it and test like so:

npm run env stop
rm -rf wordpress
unset LOCAL_SCRIPT_DEBUG
npm run build
npm run env install
npm run test-e2e

@talldan talldan self-assigned this Sep 5, 2019
@talldan talldan added the [Status] In Progress Tracking issues with work in progress label Sep 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] NUX Anything that impacts the new user experience [Package] Editor /packages/editor [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants