appsmith
appsmith copied to clipboard
fix:pagination issues in serverside pagination listv2
Description
- Fixed serverside pagination code and css
- Disabled pagination when isLoading
- Remove
onPageSizeChange
this would allow me to close #17427
Fixes #11332 Fixes #18638
Type of change
- Bug fix (non-breaking change which fixes an issue)
How Has This Been Tested?
- Cypress
Test Plan
Add Testsmith test cases links that relate to this PR
Issues raised during DP testing
Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR)
Checklist:
Dev activity
- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] PR is being merged under a feature flag
QA activity:
- [ ] Test plan has been approved by relevant developers
- [ ] Test plan has been peer reviewed by QA
- [ ] Cypress test cases have been added and approved by either SDET or manual QA
- [ ] Organized project review call with relevant stakeholders after Round 1/2 of QA
- [ ] Added Test Plan Approved label after reveiwing all Cypress test
The latest updates on your projects. Learn more about Vercel for Git ↗︎
Name | Status | Preview | Updated |
---|---|---|---|
appsmith | ✅ Ready (Inspect) | Visit Preview | Dec 29, 2022 at 8:32AM (UTC) |
@ashit-rath, accentColor
is not been used for backGround color and these wds variables were created during reskinning by @jsartisan
@Tooluloope Observation
-
On deploying List v2 with SSP enabled, the app shows error of 'no data to display' video of List v2 : https://bthrujcsw8.vmaker.com/record/nBTi9beuyWNT2CX4
-
The issue on List v1 still remains same. Error of 'no data to display' after reaching last page Video of List v1 : https://bthrujcsw8.vmaker.com/record/oqYd1AcWsqsADoe4
QQ: This PR will also fix the current issue in List v1 as well?
Hi @chandannkumar,
-
The first issue occured because you enabled Inifinite Scroll. I'll be pushing a fix for that now, as we're not enabling infinite scroll in this release.
-
The fix would only be for this List V2 not V1
- The first issue occured because you enabled Inifinite Scroll.
This is also happening when Infinite Scroll is disabled
observations:
- data stops rendering onPageChange during server-side pagination when Primary key is not set https://www.loom.com/share/25fea7e1f34d44bdaca13894feef6bcb
- Primary key gets deselected when last page is reached but when we do the JS toggle, the reference seems to be there https://www.loom.com/share/8abbe114318a4f649b92d8c55785d7a2 @Tooluloope
@kamakshibhat-appsmith No 2. is expected, as you can see once you toggle to JS you cant toggle back. and the reason the Primary Key goes is that there's no Data at that point
@ashit-rath should we cache the keys or is that going to be too much overhead?
@Tooluloope For number 2, I got the point why primary key goes but on the JS toggle , shouldn't the reference also be removed?
@Tooluloope Do we have enough clarity to move this forward?
@Tooluloope Do we have enough clarity to move this forward?
Sure, I wanted to finish up on the cypress Test first
Tested this PR and LGTM