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

Add the inert attribute and tweak definition of inert subtrees #7134

Merged
merged 1 commit into from
Mar 10, 2022

Conversation

nt1m
Copy link
Member

@nt1m nt1m commented Sep 30, 2021

Based on work in #4288 by Alice Boxhall.

Added the inert attribute and tweaked inert subtrees definition.

(See WHATWG Working Mode: Changes for more details.)


/acknowledgements.html ( diff )
/index.html ( diff )
/interaction.html ( diff )


/dom.html ( diff )
/index.html ( diff )
/indices.html ( diff )
/infrastructure.html ( diff )
/interaction.html ( diff )

source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
@josepharhar
Copy link
Contributor

It looks like there are WPTs here? https://wpt.fyi/results/inert
Here is the chrome bug: https://bugs.chromium.org/p/chromium/issues/detail?id=692360

@nt1m
Copy link
Member Author

nt1m commented Oct 8, 2021

@josepharhar Updated, thanks. I'm hoping to get to updating this PR in the next couple of weeks.

@bkardell
Copy link
Contributor

bkardell commented Oct 8, 2021

related #4288

source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
@annevk
Copy link
Member

annevk commented Oct 12, 2021

@nt1m did you see #4288 (review) by @rniwa?

How does this relate to @alice's work? Was/should there be some coordination?

@nt1m
Copy link
Member Author

nt1m commented Oct 12, 2021

@nt1m did you see #4288 (review) by @rniwa?

Yes, I described my rationale for using flat tree descendants in #7134 (comment)

How does this relate to @alice's work?

This is a newer version of it, I created a new PR to not have all the old comments.

Was/should there be some coordination?

There is an email thread going around about shipping inert with @emilio, @smfr, me, @chrishtr and @mfreed7. Alice is no longer involved afaik.

Copy link
Collaborator

@mfreed7 mfreed7 left a comment

Choose a reason for hiding this comment

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

This generally LGTM. It'd be good if @josepharhar also took a look, given that he's implementing it in Chromium.

@alice
Copy link
Contributor

alice commented Oct 12, 2021

There is an email thread going around about shipping inert with @emilio, @smfr, me, @chrishtr and @mfreed7. Alice is no longer involved afaik.

I'm not dead :) A little credit for the years of work I did on this would be nice.

@nt1m
Copy link
Member Author

nt1m commented Oct 13, 2021

@alice Thanks for all your work done on the topic! I did re-use some parts of #4288, but also re-wrote quite a bit of it too, I hope that's ok with you.

Didn't mean to discredit your work, but I did hear that you moved on to different projects. Feel free to leave feedback if you have any, your expertise is welcome :)

@alice
Copy link
Contributor

alice commented Oct 13, 2021

Starting an entirely new PR without any reference to the old one, and putting your name and not mine in the acknowledgements doesn't look like you didn't mean not to acknowledge me.

I trust the reviewers to do a good job reviewing the text here - I've said my piece on the design of the feature in the past, rehashing that seems pointless at this stage.

@nt1m
Copy link
Member Author

nt1m commented Oct 13, 2021

@alice I apologize for not asking before making a new PR, someone from Google during the CSSWG meeting said you were working on other projects, and I really wanted to get inert shipped over the finish line without having to iterate over a stale PR that I do not own.

If you're referring to my name in the "acknowledgements" section, this is because it's a requirement when making a first PR: https://github.com/whatwg/html/blob/main/CONTRIBUTING.md#pull-requests

Please add your name to the Acknowledgments section (search for <!-- ACKS) in your first pull request, even for trivial fixes. The names are sorted lexicographically.

Your name was already in this section before making this PR: "Alice Boxhall": https://html.spec.whatwg.org/multipage/acknowledgements.html#acknowledgments

I added credit in the description of this PR (sorry for not doing this in the first place). I'm happy to remove my name if you prefer, (and/or) iterating over the old PR. Please let me know if there's something else I can do.

source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
@annevk
Copy link
Member

annevk commented Oct 15, 2021

@nt1m thanks for the reference on using flat trees. That makes sense to me given pointer-events:none. Since you work at the same org, I take it you ran this by @rniwa as well?

@annevk
Copy link
Member

annevk commented Oct 15, 2021

Note to maintainers: let's try to make sure the final commit contains

Co-authored-by: Alice Boxhall <aboxhall@chromium.org>

at the end.

@nt1m nt1m force-pushed the inert branch 3 times, most recently from e90262b to 9ae9c17 Compare October 20, 2021 14:32
@nt1m
Copy link
Member Author

nt1m commented Oct 22, 2021

@annevk I've added the link to pointer-events: none; which I think should remove the need for an example :) Please let me know if there's anything else to address.

@annevk
Copy link
Member

annevk commented Oct 25, 2021

I thought the idea was that both pointer-events:none and this would share a definition in a css3-ui? (I also reopened the discussion on focus above.)

I still think keeping the example would be useful for readers not deeply familiar with the subject matter. It was also rather short.

source Outdated Show resolved Hide resolved
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

Overall this looks good to me. I mainly have editorial nits and suggestions.

source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
@nt1m nt1m force-pushed the inert branch 2 times, most recently from 8e04c4d to 83d37bc Compare March 1, 2022 16:35
source Show resolved Hide resolved
source Show resolved Hide resolved
@nt1m nt1m force-pushed the inert branch 4 times, most recently from daf571a to 6a1fcbc Compare March 1, 2022 18:03
@Loirooriol
Copy link
Contributor

Is there anything else to do here? I'd want this to land in order to send an intent to ship for Blink.

source Show resolved Hide resolved
source Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Show resolved Hide resolved
@nt1m nt1m force-pushed the inert branch 2 times, most recently from 5c7bc20 to a8ddca2 Compare March 7, 2022 22:19
Co-authored-by: Alice Boxhall <aboxhall@chromium.org>
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

LGTM, but @annevk has been the primary reviewer so he should confirm before we merge.

@nt1m nt1m requested a review from annevk March 9, 2022 16:36
@annevk annevk merged commit 755ae90 into whatwg:main Mar 10, 2022
@annevk annevk mentioned this pull request Mar 10, 2022
@annevk
Copy link
Member

annevk commented Mar 10, 2022

Thanks again @alice and @nt1m for making this happen! And everyone else who took part in design, review, implementation, and testing. It took a while, but the inert attribute is now officially part of HTML. 🎉

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.

None yet

9 participants