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

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.

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.

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

@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:

1. 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;
2. For the infiniteWrapperClass prop, is there has some cases need it? I want to keep API clear;
3. For the vue-class-component, I am a newcomer of TypeScript, can this problem be reproduce and will it affect other cases?
4. For the getBoundingClientRect problem in old browsers, is there has some refer posts? I cannot found it in the MDN;
5. 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;
6. For the build file update, it should be updated when the new version releasing, refer the Pull Request Guidelines;
7. For the combined PR, I think it is very hard if owner want to keep a part of features or bugfixes.

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.

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

@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 😓

1. 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.

2. 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

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

1. 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.

2. 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 :)

1. Perfect way.
2. 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?

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

1. Perfect way.

2. 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?

1. Perfect way.

2. 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.

@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?

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'.

@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?

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

read here carefullly: #184 (comment)

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?

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