appsmith
appsmith copied to clipboard
fix: Adjust Page Size and Remove Vertical Scroll
TL;DR; The PR includes some improvements to pagination on Table Widget. There are two main changes 1- Adjust the page size so it doesn't need to use a vertical scroll bar for just the last item. 2- When the user focuses on the page size the whole page number is selected so it can easily be set to the new value.
Description
The table widget for some reason includes a vertical scroll bar always. This interrupts the user experience as it creates another vertical scroll when it's not necessary. Multiple scroll makes it harder for the user to navigate through the window. The table widget already has a fixed pageSize and there is no way to change the pageSize. It is fully calculated by the height of the table. In this PR I've corrected the calculation and removed the 30% check of the last row that was added.
Secondly changing the page number required a few more clicks that I expected. The user had to click, delete and then type the new value. I've added a selection on blur so they can just click and type.
Media
https://recordit.co/lMRaObjy8O
Type of change
- Bug fix
- New feature
How Has This Been Tested?
- Manual
Checklist:
Dev activity
- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have made corresponding changes to the documentation
- [x] 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
Welcome to the Appsmith community! Thank you for your first pull request and making this project better. 🤗 Please make sure that you raise a review request so your code change does not go unnoticed.
@gurel is attempting to deploy a commit to the Appsmith Team on Vercel.
A member of the Team first needs to authorize it.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Updated |
|---|---|---|---|
| appsmith | ✅ Ready (Inspect) | Visit Preview | Dec 2, 2022 at 0:55AM (UTC) |
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected.
@gurel Thanks for opening this PR & making Appsmith a better project for everybody. Apologies that we didn't pick this up sooner for review. We'll try to review this code & merge it in as soon as we can.
@mohanarpit The DP link posted by vercel bot throws 404. Has it been deleted because the pr is stale ? Can you trigger a vercel build again?
Hello @gurel thanks for the contribution, this is a hard problem to solve. Seeing you picked this up gives up good hope.
I have a suggestion on the solution. The algorithm you have created can be tweaked further. Here's the problem with the current algorithm which is calculating if an item should be on a same page or a different one- https://www.loom.com/share/d62fc716e5ea4a599ec03d9980129f44
Please check this out and let me know if this is something you can fix, if not please let us know so we can pick this internally.
Really appreciate your time and effort here.
@dilippitchika Yeah, i didn't realize it went away when the app is relased. I'll look into it for sure.
Fixes https://github.com/appsmithorg/appsmith/issues/17857
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected.
This PR has been closed because of inactivity.