-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add type="button"
to vanilla <button>
elements
#55125
Conversation
Size Change: +23 B (0%) Total Size: 1.65 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.
Changes themselves all look good to me. I cross-checked against the list you provided and looks like you got them all 👍 I'm happy to test specific components when you have those testing instructions ready as well 🙂
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 specific component testing is going well for me as well. 🚀 🚢
Flaky tests detected in 2d5cdc7. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6471116113
|
There is a test failure in
However, this is also failing on trunk, so I'm confident it isn't caused by this PR. |
What?
This PR adds a
type
attribute to various native<button>
s. Test snapshots are updated where appropriate.Resolves #46814.
Why?
In Automattic/isolated-block-editor#197 it is flagged that the button element rendered by the Button component doesn't always have
type="button"
, which leads to forms being submitted involuntarily.#46814 goes into detail around this, and identifies several places in Gutenberg that need updating to reduce the risk of this happening.
How?
This PR takes the identified list of native
<button>
s across@wordpress/components
,@wordpress/block-library
and@wordpress/edit-site
, and ensures they all have a defaulttype="button"
, allowing for overrides where appropriate.Relevant code
gutenberg/packages/block-library/src/page-list-item/edit.js
Lines 66 to 71 in f04f5c5
gutenberg/packages/block-library/src/page-list-item/edit.js
Lines 90 to 95 in f04f5c5
gutenberg/packages/components/src/color-palette/index.tsx
Lines 292 to 302 in f04f5c5
gutenberg/packages/components/src/toggle-group-control/toggle-group-control-option-base/component.tsx
Lines 145 to 152 in f04f5c5
gutenberg/packages/edit-site/src/components/block-editor/resize-handle.js
Lines 33 to 38 in f04f5c5
gutenberg/packages/edit-site/src/components/page-patterns/grid-item.js
Lines 172 to 204 in f04f5c5
Testing Instructions
All unit/e2e tests should pass. Individual component testing instructions below.
block-library/src/page-list-item/edit.js
The two buttons can be can be validated by ensuring you have a nested page, then inserting a navigation block into an edit view. They will appear as dropdown toggles.
The second should appear by default, but the first requires the
openSubmenusOnClick
attribute to be set totrue
on the component. This can be achieved by opening the editor in code view, finding the relevant navigation block, and adjusting the JSON meta block...<!-- wp:navigation {"ref":8,"openSubmenusOnClick":true} /-->
(Note, the buttons are neither actionable nor labelled, but fixing that is outside the scope of this PR.)
components/src/color-palette/index.tsx
Open StoryBook and navigate to the
ColorPalette
component. The button in question operates the custom colour picker, which should not change.components/src/toggle-group-control/toggle-group-control-option-base/component.tsx
This component is rendered by every option within the
ToggleGroupControl
control, which can be tested in Storybook.edit-site/src/components/block-editor/resize-handle.js
The resize handle can be tested, amongst other ways, by opening the site editor, then heading to
Navigation > Navigation > Edit
in the Design sidebar. Once there, the drag handle should allow you to resize the navigation portal, with both mouse and keyboard.edit-site/src/components/page-patterns/grid-item.js
Grid items show up in the editor Design sidebar under
Patterns
. Note that locked patterns are not actionable, and should have anaria-disabled
attribute set tofalse
.