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

possible solution to fix the issue #96 #150

Closed
wants to merge 5 commits into from
Closed

Conversation

aohua
Copy link

@aohua aohua commented Jan 4, 2018

Here is my fix for issue #96. Not sure if it would work or not.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 75.904% when pulling 89711b6 on AOHUA:master into d4fde73 on jasonslyvia:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 75.904% when pulling fbab437 on AOHUA:master into d4fde73 on jasonslyvia:master.

@@ -27,7 +27,7 @@ export default (node) => {
continue;
}

if (overflowRegex.test(overflow) && overflowRegex.test(overflowX) && overflowRegex.test(overflowY)) {
if (overflowRegex.test(overflow) || overflowRegex.test(overflowX) || overflowRegex.test(overflowY)) {
Copy link
Author

Choose a reason for hiding this comment

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

'LazyLoad Overflow should not detect an overflow container when only one of the scroll property is auto/scroll FAILED', I don't really get it, users may not want to use all of this 3 properties right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

See here for further information about why use && instead of ||

Copy link
Author

Choose a reason for hiding this comment

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

Thank you. That's good to know. However, this condition breaks the example in Chrome. My chrome version is Version 63.0.3239.84 (Official Build) (64-bit).

Copy link
Author

Choose a reason for hiding this comment

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

I can try to fix it as well. Thanks for your reply.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 75.904% when pulling 4535cf6 on AOHUA:master into d4fde73 on jasonslyvia:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 75.904% when pulling 5844fe9 on AOHUA:master into d4fde73 on jasonslyvia:master.

@aohua aohua closed this Jun 1, 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.

None yet

3 participants