Skip to content

feat: handle form attribute#7

Closed
maelanleborgne wants to merge 2 commits intozenstruck:1.xfrom
maelanleborgne:feat/handle-form-attribute
Closed

feat: handle form attribute#7
maelanleborgne wants to merge 2 commits intozenstruck:1.xfrom
maelanleborgne:feat/handle-form-attribute

Conversation

@maelanleborgne
Copy link

@maelanleborgne maelanleborgne commented Jun 19, 2025

Closes zenstruck/browser#166

Pretty rough implementation of a feature that is central for our project tests, and that we currently circumvent by abusing reflection methods. It would be very nice to have this feature shipped in the library.

I'll let GPT describe the PR :

Description

This pull request refactors the Form and Element classes to improve handling of form elements, particularly those with explicit form attributes. It updates the logic for identifying a form associated with an element.

Refactoring and Enhancements:

  • src/Dom/Node/Form.php: Added the findNodesForForm method to refine how form-related nodes are identified. This method accounts for both direct descendants and nodes explicitly referencing the form via the form attribute. Updated fields, buttons, and submitButtons methods to use findNodesForForm for improved accuracy.
  • src/Dom/Node/Form/Element.php: Enhanced the form method to prioritize elements with explicit form attributes before falling back to the closest form ancestor.

Test Coverage:

  • tests/Node/FormTest.php: Added unit tests to validate the behavior of forms with and without IDs, ensuring correct identification of associated fields and buttons.

@kbond
Copy link
Member

kbond commented Feb 12, 2026

Hey @maelanleborgne! Sorry for the long delay on this. I didn't know about the form attribute! I am a bit confused though. Can't the existing logic find your form? Ok, I see the reason now. You're targeting a different form than the one the element is in. I am curious in the use case of this. I got it now 😁

@smnandre
Copy link
Contributor

smnandre commented Feb 12, 2026

We use this a LOT in the work project, with form in main content, and submit buttons in an outer div.

The button can be either in another form, or more generally not in the form at all

(I may have another solution @maelanleborgne )

@kbond kbond force-pushed the feat/handle-form-attribute branch from 1905533 to f57911d Compare February 12, 2026 14:46
@kbond
Copy link
Member

kbond commented Feb 12, 2026

Thanks @maelanleborgne! I rebased, CI is green!

(I may have another solution @maelanleborgne )

@smnandre I was going to merge this and release a 1.0.0, want me to hold off?

@maelanleborgne
Copy link
Author

Hi @kbond , thanks for the reply !

Yes let's wait a little bit, I think @smnandre has a more "complete" PR incoming (adding some helper methods like root, and improving coverage even more).

@kbond
Copy link
Member

kbond commented Feb 12, 2026

Sounds good!

@smnandre
Copy link
Contributor

@kbond I suggest (if #10 or this one is merged): we then it on the work project tomorrow (won't take much time) before you tag 1.0.0 ?

@kbond
Copy link
Member

kbond commented Feb 12, 2026

I believe this can be closed as I merged #10 (let me know if I'm wrong).

@smnandre before a 1.0, I want to have our chat on Saturday!

@kbond kbond closed this Feb 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support input with form attribute

3 participants