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

[Editorial] rework button states to allow for command/commandfor attributes #10960

Conversation

keithamus
Copy link
Contributor

@keithamus keithamus commented Jan 29, 2025

This PR changes the way button elements represent their state; specifically how they are defined as submit buttons. This also re-writes some of the generic algorithms (such as the IDL reflection) to be manual steps.

The intent of this change is to carve out the space for the new command and commandfor attributes. Most of these changes are already in #9841 but this PR separates them out to shrink the amount of changes inside #9841.


/form-control-infrastructure.html ( diff )
/form-elements.html ( diff )
/forms.html ( diff )
/input.html ( diff )
/nav-history-apis.html ( diff )
/popover.html ( diff )
/semantics-other.html ( diff )

@keithamus keithamus requested review from domenic and annevk February 2, 2025 12:03
keithamus added a commit to keithamus/html that referenced this pull request Feb 2, 2025
lukewarlow

This comment was marked as outdated.

@keithamus keithamus force-pushed the rework-button-states-to-allow-for-command-commandfor-attributes branch from 5dea222 to 211e3b8 Compare February 8, 2025 21:33
@keithamus
Copy link
Contributor Author

Hey @foolip I've addressed your feedback, would be great to get another review pass if you have time?

Reset doesnt return! Keeping this editorial only is important
@mfreed7
Copy link
Contributor

mfreed7 commented Feb 10, 2025

This PR introduces a breaking change to popovertarget button behaviour and I want to bring this up for wider discussion.

Two questions: is this behavior change intentional? (Side-question, if yes: why?) And most importantly, if the intention is to try to land this behavior change, has someone done the compat analysis? I'd be a bit concerned.

When we landed popover initially, the desired behavior was that the popover gets triggered unless the button would otherwise take a form action. That's roughly category 1. We didn't restrict category 2.

@lukewarlow
Copy link
Member

Apologies I should have updated my comment it's no longer applicable. Keith has fixed the changes.

I should note that the spec does currently differ from chromium but that's a separate discussion to be had imo.

@foolip foolip merged commit 69110cb into whatwg:main Feb 11, 2025
2 checks passed
@foolip
Copy link
Member

foolip commented Feb 11, 2025

Thanks @keithamus! I hope rebasing on top of this won't be too much of a hassle 😅

@keithamus keithamus deleted the rework-button-states-to-allow-for-command-commandfor-attributes branch February 11, 2025 15:42
@@ -45861,8 +45861,8 @@ interface <dfn interface>HTMLTableCellElement</dfn> : <span>HTMLElement</span> {

<p>Some <span data-x="category-submit">submittable elements</span> can be, depending on their
attributes, <dfn data-x="concept-button">buttons</dfn>. The prose below defines when an element
is a button. Some buttons are specifically <dfn export data-lt="submit button"
data-x="concept-submit-button">submit buttons</dfn>.</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how you can move this definition to the button element section. The button element is not alone in being able to be a submit button.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, this swap of definition and use was my suggestion in #10960 (comment). I didn't check other uses of the concept and now see that it's used a lot to make various things "specifically a submit button."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants