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

feat(infinite-scroll): add custom trigger #285

Merged
merged 14 commits into from
Jun 24, 2024

Conversation

caesarWHLee
Copy link
Collaborator

Notable Change

  • Add custom trigger mode to let user decide where the trigger point should be set.
  • Update dev examples.
  • Update Readme.md.

Follow-up

After PR approved, new npm package will be pushed with new version.

Copy link
Contributor

Choose a reason for hiding this comment

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

  1. customTrigger 應該調整命名為 hasCustomTrigger,使其有更明確的 boolean 值意涵
  2. loaderElement 可以是 customTriggerRef.currentloaderRef.current,簡化 useEffect 的處理,同時讓在 hasCustomTriggertrue 的情況下,也可以支援手動載入

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  1. 修改在 ed41865
  2. 合併使用 triggerRef,不過 customTriggerRef 不應該支援手動載入(目前的設計會造成 listing element 點擊意外觸發 loadmore 效果,可能會混淆 element 點擊作用),改動在 ac56405

>
{Object.entries(ExampleType).map(([key, type]) => {
return (
<button
Copy link
Contributor

Choose a reason for hiding this comment

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

可以增加 active 效果,以作為辨識

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

修改在 fed01b6

| loader | `ReactNode` | `false` | The loader element to be displayed during data loading |

| customTrigger | `boolean` | `false` | Wether the custom trigger ref will provided throught children callback to set up trigger point |
Copy link
Contributor

Choose a reason for hiding this comment

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

will be provided

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

修改在 ed41865

@@ -44,6 +44,13 @@ You should ensurce T in `initialList`, `fetchListInPage` and `children` that rep
1. no more data
2. amount of items reached `Math.min(amountOfElements, pageAmount * pageSize)`

### Trigger loadmore
- Default (customTrigger: false)
- This component will insert a loading `<div/>` to trigger intersection which means it only starts to load more at the end of the showing list
Copy link
Contributor

Choose a reason for hiding this comment

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

This component appends an element which triggers load-more effect at the end of the element list by default

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

修改在 ed41865

- Default (customTrigger: false)
- This component will insert a loading `<div/>` to trigger intersection which means it only starts to load more at the end of the showing list
- Custom Trigger (customTrigger: true)
- Set the prop `customTrigger` to `true` and use the `customTriggerRef` as the second param from the `children` callback to set to the desired elemnt as trigger point. (check [custom-trigger-example](./dev/components/custom-trigger-example.tsx))
Copy link
Contributor

Choose a reason for hiding this comment

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

the second parameter -> the second augument

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

修改在 ed41865

- Default (customTrigger: false)
- This component will insert a loading `<div/>` to trigger intersection which means it only starts to load more at the end of the showing list
- Custom Trigger (customTrigger: true)
- Set the prop `customTrigger` to `true` and use the `customTriggerRef` as the second param from the `children` callback to set to the desired elemnt as trigger point. (check [custom-trigger-example](./dev/components/custom-trigger-example.tsx))
Copy link
Contributor

Choose a reason for hiding this comment

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

to set to the desired elemnt as trigger point -> to designate element as trigger point

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

修改在 ed41865

Copy link
Contributor

@erase2004 erase2004 left a comment

Choose a reason for hiding this comment

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

  1. 不建議在此 package 的 dev dependecy 加上 styled-component,應該讓這個 package 盡可能沒有 styling 的行為,如果要用上 styled-component,可以放在 root (ver: 5.3.5)
  2. hasCustomTriggertrue 的情況下,isAutoFetch 必須為 true 的話,可以用 ts-xor 來定義 props,讓使用端清楚這限制

@@ -18,5 +18,5 @@
"outDir": "dist",
"declarationMap": true
},
"include": ["src/**/*"]
"include": ["src/**/*", "dev/**/*"]
Copy link
Contributor

Choose a reason for hiding this comment

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

這邊要改的話,記得調整 package.json 的內容,因為輸出 type 的結構有變

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

感謝提醒,先拿掉好了,修改在 b49ee49

@caesarWHLee
Copy link
Collaborator Author

  1. 不建議在此 package 的 dev dependecy 加上 styled-component,應該讓這個 package 盡可能沒有 styling 的行為,如果要用上 styled-component,可以放在 root (ver: 5.3.5)

修改在 48f1d2b

  1. hasCustomTriggertrue 的情況下,isAutoFetch 必須為 true 的話,可以用 ts-xor 來定義 props,讓使用端清楚這限制

修改在 e5867ae

/** Wether the custom trigger ref will provided throught children callback to set up trigger point */
hasCustomTrigger?: boolean
}
} & XOR<CustomTrigger, AutoFetch>
Copy link
Contributor

Choose a reason for hiding this comment

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

我會弄成以下樣子

XOR<{
  hasCustomTrigger: true
  isAuthFetch?: true
}, {
  hasCustomTrigger?: false
  isAuthFetch?: boolean
}>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

修改在 ae7aac3

package.json Outdated
@@ -52,6 +52,8 @@
"lint-staged": "^12.4.2",
"prettier": "^2",
"regenerator-runtime": "^0.13.9",
"styled-components": "^6.1.11",
"ts-xor": "^1.3.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

ts-xor 應該放到 package 的 dev dependecy,而非 root,這樣使用端在安裝後,才能夠正確解析 prop type

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

修改在 bf19928

package.json Outdated
@@ -52,6 +52,8 @@
"lint-staged": "^12.4.2",
"prettier": "^2",
"regenerator-runtime": "^0.13.9",
"styled-components": "^6.1.11",
Copy link
Contributor

Choose a reason for hiding this comment

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

可以先固定在 5.3.5 的版本,畢竟其他套件也是用這個版本

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

修改在 bf19928

- fix styled-components version to 5.3.5
- move ts-xor package into infinite-scroll-list package
Copy link
Contributor

@erase2004 erase2004 left a comment

Choose a reason for hiding this comment

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

LGTM 👏
記得更新一下 CHANGELOG

@erase2004
Copy link
Contributor

喔對了,package 自己的 package.json 也要更新一下版號

@caesarWHLee
Copy link
Collaborator Author

  • 更新 package version 為 1.1.0 - c3cefd8
  • 更新 changelog - 175b263
  • 已 publish 到 npm

@caesarWHLee caesarWHLee merged commit abc8659 into readr-media:main Jun 24, 2024
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

2 participants