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

Specifying visual viewport for root intersection rect and clarifying … #128

Closed
wants to merge 21 commits into from
Closed

Conversation

ChumpChief
Copy link
Member

…coordinate space for rootMargin

This PR corresponds to issue #95 @szager-chromium @bokand @RByers

szager-chromium and others added 21 commits February 24, 2016 20:47
This should integrate better with the existing html event loop spec.
Use a document-centric processing model.
The concept of a node being "deleted", as opposed to merely being
detached from the DOM, is not specified by any standard, so we
shouldn't refer to it here.
Don't specify behavior of observer with invalid root.
Incorrectly used undefined `element` property (out of date?), undefined variable and syntax error (stray `{`).
Previous code used for-in on an array, which isn't recommended. Changed to for-of.
Correct unreliable checks for properties w/ more reliable ones.
Use CSS classes on container and inner-scroll-surface. Fix invalid rootMargin.
</h3> -> </h4>
@mpb
Copy link
Collaborator

mpb commented May 6, 2016

LGTM

@ChumpChief
Copy link
Member Author

When can this be merged? Anything else you need from me to help get this in?

@szager-chromium
Copy link
Collaborator

I'd prefer it if we could define the option to select layout vs. visual viewport at the same time as we change the language, and the language should probably use "visual or layout viewport" as appropriate.

@ChumpChief
Copy link
Member Author

Can we treat that as a separate, augmenting change following this one up? I think the addition of an option probably needs more discussion (e.g. see my and David's comments on #95) and I don't think it results in any change to the default behavior, which is what this issue/PR is focused on.

@yoavweiss
Copy link
Contributor

@ChumpChief - Apologies for letting this slide for so long, but could you join the WICG in order to appease the IPR bots?

@siusin
Copy link
Contributor

siusin commented Sep 12, 2017

Please merge the requested change to the master branch (index.bs) once the group/editors reach a consensus on this issue, the travis bot will generate an index.html with the latest version of Bikeshed and push it to gh-pages.

Our apologies for the inconvenience.

@yoavweiss
Copy link
Contributor

@ChumpChief - friendly ping :) Can you join the WICG so the IPR bots would be happy and green?

@ChumpChief
Copy link
Member Author

Wow, old one :) I think I've linked it now, let me know if there's still any issue.

@yoavweiss
Copy link
Contributor

@ChumpChief Yeah, dug it from the bottom of my inbox... :D repo-manager is still not happy. Not sure why

@siusin
Copy link
Contributor

siusin commented Jan 3, 2018

@ChumpChief could you try linking your Github account with your W3C account? Also, please remove the .html file from this pull request? the .travis bot will generate the document for you once you submit the .bs file :)

(Thanks @yoavweiss ;)

@ChumpChief
Copy link
Member Author

I linked it yesterday as noted. Shows up correctly on the connectedaccounts page as well.

@siusin
Copy link
Contributor

siusin commented Jan 4, 2018

Thanks for the update @ChumpChief .

@dontcallmedom the IRP checker reported an error when we tried to revalidate a PR after migrating the repo from WICG... any suggestion?

Unknown repository: WICG/IntersectionObserver

@dontcallmedom
Copy link
Member

@siusin fixed - at the moment, the IPR checker doesn't handle correctly handling of existing Pull Requests after migration of a repo, so this has to be fixed manually.

@siusin
Copy link
Contributor

siusin commented Jan 9, 2018

Thanks @dontcallmedom !

@ChumpChief
Copy link
Member Author

@yoavweiss Looking to wrap this one up -- anything you need me to do?

@ChumpChief ChumpChief closed this Nov 15, 2019
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.