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

Fix responsive issues due to using scrollWidth instead of clientWidth… #20

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

billschaller
Copy link
Collaborator

… - paddingLeft - paddingRight.

el[0].scrollWidth is definitely not the same as el.width().

scrollWidth returns the larger of either the width of the container or the width of the scrolling context, which is the width of the existing content in the element. Therefore, this change completely breaks responsive containers when resizing to a smaller width. Since the width of the content is larger than the new width of the container, this is not useful for sizing responsive containers.

The changes changing el.width() to el[0].scrollWidth should all be reverted. I've been trying to track the cause of responsive issues down for weeks off and on, and finally dedicated enough time today to find this...

@heartdisease
Copy link

Is there a reason why this pull request has never been merged?

@dsmorse
Copy link
Owner

dsmorse commented May 19, 2016

initially It was because it conflicted with the webcomponents.js, but that has become moot, now it's because there is a merge conflict.

@fixmycode
Copy link

@billschaller I know this is an old PR but would it be possible to discard the changes made at dist? dist is used so that upstream can offer an already built package to package managers, and should reflect the state of upstream. Other PR's could've changed that state before yours so it wouldn't be right to overwrite those changes.

@fixmycode fixmycode self-assigned this Jul 19, 2018
@fixmycode fixmycode removed their assignment Aug 8, 2020
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.

4 participants