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

fix: space above pagination dots and pagination bug #155

Merged
merged 2 commits into from
Apr 9, 2024

Conversation

sunnyguduru
Copy link
Contributor

@sunnyguduru sunnyguduru commented Apr 9, 2024

I removed the extra space between the choose sdk list and the pagination dots.
CleanShot 2024-04-09 at 15 56 58@2x

bug fix: we were only showing 15 items total. This was due to setting pagination.PerPage manually to 5. We can't manually control this property without bugs due to how list.updatePagination() works. Setting the list height to 9 shows us 5 items, and displays all 24-25 items.

video: https://share.cleanshot.com/bs9YgdBF

@@ -40,7 +40,6 @@ func NewChooseSDKModel(selectedIndex int) tea.Model {
l.SetShowPagination(true)
l.SetShowStatusBar(false)
l.SetFilteringEnabled(false) // TODO: try to get filtering working
l.Paginator.PerPage = 5
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're not allowed to manually set this unfortunately. The property gets computed automatically by list.updatePagination(). So setting it manually messes up the computation. I found a way to show exactly 5 items or less on every page though by setting the hight on line 34 to 9.

@sunnyguduru sunnyguduru changed the title bug: fix choose sdk pagination spacing and bug fix: space above pagination dots and pagination bug Apr 9, 2024
@sunnyguduru sunnyguduru merged commit adbc53c into main Apr 9, 2024
2 of 3 checks passed
@sunnyguduru sunnyguduru deleted the sunny/sc-239311/change-sdk-pagination-dots branch April 9, 2024 23:24
@sunnyguduru sunnyguduru mentioned this pull request Apr 15, 2024
3 tasks
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.

2 participants