Skip to content

Make forms work in shadow trees#1135

Merged
annevk merged 3 commits into
masterfrom
shadow-form
May 11, 2016
Merged

Make forms work in shadow trees#1135
annevk merged 3 commits into
masterfrom
shadow-form

Conversation

@annevk

@annevk annevk commented Apr 27, 2016

Copy link
Copy Markdown
Member

No description provided.

@annevk

annevk commented Apr 27, 2016

Copy link
Copy Markdown
Member Author

I suspect the submit event needs to be scoped. That's not done yet. Not sure about other form-specific events...

Seems submit is not mentioned by https://w3c.github.io/webcomponents/spec/shadow/#scoped-flag but that's probably a bug.

@hayatoito, @smaug----, thoughts?

@smaug----

Copy link
Copy Markdown

It probably should indeed. Otherwise the light DOM may get odd submit events which aren't coming from any form. Same with reset.

Not sure about other events like input/change, since one could implement a new form-widget-like element using shadow DOM and such element might be used outside any form, just like and such.

@annevk

annevk commented Apr 27, 2016

Copy link
Copy Markdown
Member Author

This also fixes #833.

Note that submit does cause the whole document to navigate so in that sense the outer tree might like to know about it, but I guess they can also listen to beforeunload...

@smaug----

Copy link
Copy Markdown

I was thinking that but it isn't much different to some script in shadow dom doing location.href = "somenewpage".

@hayatoito

Copy link
Copy Markdown
Member

LGTM.
Regarding a submit event, there is no intention to make it non-scoped.
Making it scoped sounds reasonable to me.

@annevk

annevk commented Apr 28, 2016

Copy link
Copy Markdown
Member Author

FYI, @rniwa.

@domenic domenic added the topic: shadow Relates to shadow trees (as defined in DOM) label Apr 28, 2016
@annevk

annevk commented Apr 29, 2016

Copy link
Copy Markdown
Member Author

I annotated the "invalid" event as needing to be scoped, though maybe that's in the same realm as "input" and "change"...

@annevk

annevk commented May 3, 2016

Copy link
Copy Markdown
Member Author

This also fixes #283 it seems. Yay.

@annevk annevk added the do not merge yet Pull request must not be merged per rationale in comment label May 3, 2016
@annevk

annevk commented May 3, 2016

Copy link
Copy Markdown
Member Author

Once we have figured out scoped vs unscoped this can probably land with some slight changes. Making it clear we should wait a little bit for now.

@annevk

annevk commented May 10, 2016

Copy link
Copy Markdown
Member Author

I removed the changes that make this resolve #833. I also removed the scoped event annotations now they are scoped by default.

@annevk

annevk commented May 10, 2016

Copy link
Copy Markdown
Member Author

Still LGTM?

This fixes #283 and #1160. It also fixes the remaining bit of #762 now events are scoped by default.

@annevk annevk removed the do not merge yet Pull request must not be merged per rationale in comment label May 10, 2016
@hayatoito

Copy link
Copy Markdown
Member

Yeah, LGTM.

@annevk
annevk merged commit 927fda0 into master May 11, 2016
@annevk
annevk deleted the shadow-form branch May 11, 2016 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic: shadow Relates to shadow trees (as defined in DOM)

Development

Successfully merging this pull request may close these issues.

4 participants