Skip to content

Conversation

@a-kriya
Copy link
Contributor

@a-kriya a-kriya commented May 17, 2017

this.$el.offsetTop will always evaluate to 0 when this.$el is inside a td (but the scrolling element is a table). This causes the distance to keep decreasing and the component continuously calls onInfinite.

This PR fixes the issue by explicitly calculating the top offset of this.$el, relative to the actual scrolling element, and not just the direct parent.

I tried to see if this.$el.offsetTop will be updated by just setting CSS position to relative on tr, td, and this.$el, and all combinations, but no luck there. The performance hit of this PR will be an extra getBoundingClientRect call for this.$el.

`this.$el.offsetTop` will always evaluate to 0 when `this.$el` is inside a `td` (but the scrolling element is a `table`).

This PR fixes the issue by explicitly calculating the top offset of `this.$el`, relative to the actual scrolling element, and not just the direct parent.

I tried to see if `this.$el.offsetTop` will be updated by just setting CSS `position` to `relative` on `tr`, `td`, and `this.$el`, and all combinations, but no luck there. The performance hit of this PR  will be an extra `getBoundingClientRect` call for `this.$el`.
@codecov
Copy link

codecov bot commented May 17, 2017

Codecov Report

Merging #48 into master will decrease coverage by 3.5%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #48      +/-   ##
==========================================
- Coverage     100%   96.49%   -3.51%     
==========================================
  Files           1        1              
  Lines          56       57       +1     
  Branches       12       12              
==========================================
- Hits           56       55       -1     
- Partials        0        2       +2
Impacted Files Coverage Δ
src/components/InfiniteLoading.vue 96.49% <100%> (-3.51%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7af8b32...dba7ece. Read the comment docs.

@PeachScript
Copy link
Owner

Very clean and neat way, thank you very much!

@PeachScript PeachScript merged commit f4623af into PeachScript:master May 17, 2017
@a-kriya a-kriya deleted the patch-1 branch May 17, 2017 18:11
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.

2 participants