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

scrollLeft/scrollTop, why do they exist? #2140

Closed
syranide opened this issue Sep 4, 2014 · 5 comments
Closed

scrollLeft/scrollTop, why do they exist? #2140

syranide opened this issue Sep 4, 2014 · 5 comments

Comments

@syranide
Copy link
Contributor

syranide commented Sep 4, 2014

scrollLeft and scrollTop are specified in HTMLDOMPropertyConfig, I understand what they're probably for. But what is the use-case? Doesn't it make more sense to just let users set/update them manually (on DOM nodes)?

I'm asking because we currently emit invalid attributes for them, we also don't set the properties when reusing server-markup, so they seem really really hacky right now.

cc @zpao @spicyj @yungsters

@sophiebits
Copy link
Collaborator

(I've no idea.)

@zpao
Copy link
Member

zpao commented Sep 8, 2014

Looking back at pre open source history this blames to when @yungsters created the property whitelist. I can't see a good reason it was added, perhaps there was some legacy reason which made sense at the time. I can't find any consumers on DOM components, only composite components so I think it should be fine to remove them.

@lojjic
Copy link

lojjic commented Sep 10, 2014

I've come across a potential use case for this, where I want to update the scroll position of an element and I want it to happen in conjunction with a bunch of other DOM writes. I can of course set it manually on the DOM after the render update, but it's much harder to keep unwanted reads from happening in between.

It actually appears to work at a basic level, but easily gets out of sync with the actual scroll position if it's changed by the user.

@zpao
Copy link
Member

zpao commented Sep 10, 2014

@lojjic There are other properties you can set on a DOM node that we don't expose. Proper setting of scroll* should be done directly on the node (the getting out of sync is a very good reason).

@syranide
Copy link
Contributor Author

Closing in favor of PR #2202.

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

No branches or pull requests

4 participants