* [bug] need to restore the scroll position when direction is top #184

snowyu · 2018-09-29T13:08:47Z

fixed the bug infinite-loading dead loop when the direction is top

add a infiniteWrapperClass prop to force infinite wrapper.

fixed bug can not used with vue-class-component(ts)

Sorry I am too lazy to split them. So all in it.

codecov · 2018-09-29T13:12:30Z

Codecov Report

Merging #184 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #184   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           2      2           
  Lines         106    124   +18     
  Branches       25     31    +6     
=====================================
+ Hits          106    124   +18

Impacted Files	Coverage Δ
src/components/InfiniteLoading.vue	`100% <100%> (ø)`	⬆️

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 c64c9d7...5ca946f. Read the comment docs.

snowyu · 2018-10-01T02:42:32Z

Known Issue:

The getBoundedRect().height and clientHeight are always return scrollHeight instead of viewport height In the old Browser(below Android 7).
element.scrollTop can not work on the old browser.
- only document.body.scrollTop can work.

snowyu · 2018-10-01T10:35:19Z

Ok, I've found the height must be exists in the old browser if you wanna work around above issues.

PeachScript · 2018-10-08T02:37:22Z

@snowyu thanks for these detailed contributions and full tests! I have some questions and suggestions for these commit, please don't mind if there has something wrong:

For the infinite-loading dead loop, it is a missing feature(see direction property in the doc) rather than a bug in this version, and I plan to support it in the next version, can be find in the Project Card;
For the infiniteWrapperClass prop, is there has some cases need it? I want to keep API clear;
For the vue-class-component, I am a newcomer of TypeScript, can this problem be reproduce and will it affect other cases?
For the getBoundingClientRect problem in old browsers, is there has some refer posts? I cannot found it in the MDN;
For the element.scrollTop, exactly it is not a common property for every element, it will be fallback to the pageYOffset if scrollTop property not a number;
For the build file update, it should be updated when the new version releasing, refer the Pull Request Guidelines;
For the combined PR, I think it is very hard if owner want to keep a part of features or bugfixes.

snowyu · 2018-10-08T08:33:08Z

For the infiniteWrapperClass prop, is there has some cases need it? I want to keep API clear;

The q-scroll-area component in the Quasar framework will create a sub scroll-container in it.
and mark it as the scroll class.

For the vue-class-component, I am a newcomer of TypeScript, can this problem be reproduce and will it affect other cases?

Use the vue-class-component, you can write component more pretty.

import InfiniteLoading, { StateChanger } from 'vue-infinite-loading';

@Component({
  components: { ChatRoomInfo, InfiniteLoading }
})
export default class ChatRoom extends Vue {
  loadMore($state: StateChanger) {
    console.log('loadMore')
    if (++this.count > 5) return $state.complete();
   ...
    this.$nextTick(()=>$state.loaded());
  }
}

For the getBoundingClientRect problem in old browsers, is there has some refer posts? I cannot found it in the MDN;

Yes, I'd searched a long time too, but found little. You can try it on the android emulator (such as API 19). The scrollTop and clientHeight do not work, once you remove the height style.

<template lang="pug">
  div(style="width: 100%; max-width: 96vw;")
    q-scroll-area(ref="scrollWrapper" style="height: calc(100vh);")
      infinite-loading(
        @infinite="loadMore" direction="top" :distance="0" infinite-wrapper-class="scroll"
        :top-scroll-pos="10"
      )
      q-chat-message(
        v-for="(msg, index) in messages"
        :key="`reg-${index}`"
        :label="msg.label"
        :sent="msg.sent"
        :text-color="msg.textColor"
        :bg-color="msg.bgColor"
        :name="msg.name"
        :avatar="msg.avatar"
        :text="msg.text"
        :stamp="msg.stamp"
      )
</template>

6,7
I am very sorry, and very grateful thanks for your pretty component. This is why I share mime.

snowyu · 2018-10-08T11:28:02Z

BTW: You can use this as typescript starter to test: https://github.com/snowyu/quasar-ts-seed.

PeachScript · 2018-10-11T02:31:31Z

@snowyu thanks for your explanation and glad you like this project!

For the infiniteWrapperClass prop, how do you think if the forceUseInfiniteWrapper prop support to pass a string value as the infinite wrapper class name?

And I tried to use vue-class-component as your said, it is very great! But the autocomplete worked properly, so what is the bug using with vue-class-component?

I will check the getBoundingClientRect problem later, I have no Android Studio env now 😓

snowyu · 2018-10-11T05:44:43Z

Following the component meaning, the forceUseInfiniteWrapper as string should be customized to the attribute name of element instead of the default "infinite-wrapper" attribute name to keep compatibility.

And maybe we need the similar infiniteWrapperId prop to locate the wrapper via the element id.

typescript declaration problem

2.1 V2.3.3

// used in npm repository.
export = InfiniteLoading;

https://www.typescriptlang.org/docs/handbook/modules.html

They also support replacing the exports object with a custom single object. Default exports are meant to act as a replacement for this behavior; however, the two are incompatible. TypeScript supports export = to model the traditional CommonJS and AMD workflow.

This is not a ts way. So you must enable "allowSyntheticDefaultImports": true in the tsconfig.json file:

2.2 master branch

// used in master branch
export default InfiniteLoading;

This only exports the InfiniteLoading class. not the StateChanger etc in the namespace.

And the InfiniteLoading namespace(internal module name) is the same with class name confused me: What is really exported, namespace or class?

BTW: I suggest you try the vue-test-utils with jest to test. It has a cool vscode plugin: https://github.com/jest-community/vscode-jest

snowyu · 2018-10-11T06:13:08Z

Forget vue-test-utils, it is hard to test scroll. but jest is good.

PeachScript · 2018-10-11T06:18:18Z

If the string value will pass to the document.querySelector method, is it has more compatibility? The boolean value means use the default infinite-wrapper attribute, the string value means find the specific element through selector.
I see, it is a export bug in v2.3.3, and it has been fixed by another contributor with this commit, but I am busy to rewrite the document and forgot to release a new version. Currently, the v2.3.4 has been released, contains that commit, this bug should be fixed.

I will try the Jest framework, good idea :)

snowyu · 2018-10-11T07:18:25Z

Perfect way.
It is still wrong in the current master branch.

vue-infinite-loading/types/index.d.ts

Line 50 in c64c9d7

export default InfiniteLoading;

This only exports the InfiniteLoading class. not the StateChanger etc in the namespace.
And the InfiniteLoading namespace(internal module name) is the same with class name confused me: What is really exported, namespace or class?

…loading

snowyu · 2018-10-11T08:33:07Z

BTW I add a chat demo to my quasar-ts-starter-seed

rustkt · 2018-10-11T10:34:37Z

Perfect way.
It is still wrong in the current master branch.
  [vue-infinite-loading/types/index.d.ts](https://github.com/PeachScript/vue-infinite-loading/blob/c64c9d70bcccf8c4643c1ca904b89ae5e47df1ef/types/index.d.ts#L50)


     Line 50
  in
  [c64c9d7](/PeachScript/vue-infinite-loading/commit/c64c9d70bcccf8c4643c1ca904b89ae5e47df1ef)





    
      
       export default InfiniteLoading;
This only exports the InfiniteLoading class. not the StateChanger etc in the namespace.
And the InfiniteLoading namespace(internal module name) is the same with class name confused me: What is really exported, namespace or class?

Hi, I improved types definition and send a PR just now, could you please review this change?

rustkt · 2018-10-11T10:44:37Z

Perfect way.
It is still wrong in the current master branch.
  [vue-infinite-loading/types/index.d.ts](https://github.com/PeachScript/vue-infinite-loading/blob/c64c9d70bcccf8c4643c1ca904b89ae5e47df1ef/types/index.d.ts#L50)


     Line 50
  in
  [c64c9d7](/PeachScript/vue-infinite-loading/commit/c64c9d70bcccf8c4643c1ca904b89ae5e47df1ef)





    
      
       export default InfiniteLoading;
This only exports the InfiniteLoading class. not the StateChanger etc in the namespace.
And the InfiniteLoading namespace(internal module name) is the same with class name confused me: What is really exported, namespace or class?

Sorry, missed changes from your PR, it has already been improved.

PeachScript · 2018-10-11T11:54:44Z

@snowyu so efficient work! Could you create another PR only contains the infinite-wrapper improvement?

About the TypeScript definition, I try to search in the official documentation, I think the StateChanger also be exported through the declaration-merging feature, is that right?

snowyu · 2018-10-11T13:40:41Z

About the TypeScript definition just try my demo after changing this line.

//package.json
    //"vue-infinite-loading": "github:snowyu/vue-infinite-loading",
    "vue-infinite-loading": "github:PeachScript/vue-infinite-loading",

first u should install quasar-cli : npm install -g vue-cli

then run it:

quasar dev

You should get the error:

 ERROR  Failed to compile with 1 errors                                                                                                                                              9:26:07 PM

 error  in quasar-ts-seed/src/components/ChatMessages.vue.ts

[tsl] ERROR in quasar-ts-seed/src/components/ChatMessages.vue.ts(25,27)
      TS2305: Module '"quasar-ts-seed/node_modules/vue-infinite-loading/types/index"' has no exported member 'StateChanger'.

PeachScript · 2018-10-12T02:23:45Z

@snowyu I have been reproduced this problem with TypeScript-Vue-Starter, but when I use v2.3.3 with export = InfiniteLoading statement, it works well, what is the different for TypeScript between export = InfiniteLoading and export default InfiniteLoading?

PeachScript · 2018-10-12T02:27:32Z

The v2.3.4 is a problem version, I have already deprecated it.

snowyu · 2018-10-12T02:57:59Z

read here carefullly: #184 (comment)

PeachScript · 2018-10-12T03:23:55Z

Oh sorry I missed it, thanks for your patience! It would be better if you can create another PR about the TypeScript definition fixes :P

And I plan to release a new version contains the forceUseInfiniteWrapper improvement and the definition fix. In additional, the full top direction feature will be added in the next minor version, will it make trouble for your job?

PeachScript · 2018-10-24T08:24:21Z

The v2.4.0 has been released, contains full feature for top direction, I'm sorry that I cannot merge this PR because the component has changed a lot, thanks for your contributions.

Best Regards

snowyu added 5 commits September 29, 2018 20:51

chore: need to restore the scroll position when direction is top

54f73fc

chore: add an infiniteWrapperClass property to InfiniteLoading

96c6ea3

refactor: fixed ts delcartion can not work with vue-class-component

33ec0c1

build: restoreScrollPos and infiniteWrapperClass prop

b205179

Merge branch 'master' of github.com:snowyu/vue-infinite-loading

3994b43

snowyu added 5 commits September 30, 2018 08:22

* chore: forget remember lastHeight when direction is top

455d4d8

chore: should use the clientHeight instead of boundingClientRect

0df1ce5

chore: add test for restoreScrollPos

5254fb6

build: update dist too

cd7b36c

build: update dist too

b17d7b0

snowyu added 5 commits October 2, 2018 09:26

chore: fixed hasClass should return false when no infiniteWrapperClass

501de83

chore: add the topScrollPos prop to check pos when scroll top

ed926e6

build: add the topScrollPos

98d31c6

chore: add test topScrollPos > 1

77702af

fix: ts declaration file

1ebc7ce

snowyu added 2 commits October 11, 2018 15:57

chore: forceUseInfiniteWrapper prop could be a css seletor now

ce2b8c9

Merge branch 'master' of https://github.com/PeachScript/vue-infinite-…

5ca946f

…loading

snowyu mentioned this pull request Oct 11, 2018

feat: the forceUseInfiniteWrapper could be a css selector now #187

Merged

snowyu mentioned this pull request Oct 12, 2018

fix: ts declaretion export the default class and types #188

Merged

PeachScript closed this Oct 24, 2018

* [bug] need to restore the scroll position when direction is top #184

* [bug] need to restore the scroll position when direction is top #184

Uh oh!

Conversation

snowyu commented Sep 29, 2018

Uh oh!

codecov bot commented Sep 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

snowyu commented Oct 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

snowyu commented Oct 1, 2018

Uh oh!

PeachScript commented Oct 8, 2018

Uh oh!

snowyu commented Oct 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

snowyu commented Oct 8, 2018

Uh oh!

PeachScript commented Oct 11, 2018

Uh oh!

snowyu commented Oct 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

snowyu commented Oct 11, 2018

Uh oh!

PeachScript commented Oct 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

snowyu commented Oct 11, 2018

Uh oh!

snowyu commented Oct 11, 2018

Uh oh!

rustkt commented Oct 11, 2018

Uh oh!

rustkt commented Oct 11, 2018

Uh oh!

PeachScript commented Oct 11, 2018

Uh oh!

snowyu commented Oct 11, 2018

Uh oh!

PeachScript commented Oct 12, 2018

Uh oh!

PeachScript commented Oct 12, 2018

Uh oh!

snowyu commented Oct 12, 2018

Uh oh!

PeachScript commented Oct 12, 2018

Uh oh!

PeachScript commented Oct 24, 2018

Uh oh!

Uh oh!

codecov bot commented Sep 29, 2018 •

edited

Loading

snowyu commented Oct 1, 2018 •

edited

Loading

snowyu commented Oct 8, 2018 •

edited

Loading

snowyu commented Oct 11, 2018 •

edited

Loading

PeachScript commented Oct 11, 2018 •

edited

Loading