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 picker won't scroll down when it hits the bottom #1544 #1546

Closed
wants to merge 1 commit into from

Conversation

hahanein
Copy link
Contributor

@hahanein hahanein commented Jan 19, 2022

This fixes #1544

@k2d222
Copy link
Contributor

k2d222 commented Jan 19, 2022

a / b * c vs a / (b * c).
try let offset = self.cursor / std::cmp::max(1, (rows as usize)) * (rows as usize);

@hahanein
Copy link
Contributor Author

hahanein commented Jan 19, 2022

try let offset = self.cursor / std::cmp::max(1, (rows as usize)) * (rows as usize);

That would alter the result in cases in which rows <= 1

I'm assuming that's correct regardless because this state is not useful anyway?

Amended the change.

@k2d222
Copy link
Contributor

k2d222 commented Jan 19, 2022

looking back at the code, this would be my favourite solution:
let offset = self.cursor - (self.cursor % rows as usize);
Its also a bit clearer what it does in my opinion.

@hahanein
Copy link
Contributor Author

Works for me 👍🏻

@k2d222
Copy link
Contributor

k2d222 commented Jan 19, 2022

Not for me haha :) sorry I didn't know the remainder operation also fails with 0. Third time's the charm ;)
self.cursor - (self.cursor % std::cmp::max(1, rows as usize));
of self.cursor / std::cmp::max(1, rows as usize) * (rows as usize)

@hahanein
Copy link
Contributor Author

hahanein commented Jan 19, 2022

I quit (math) 👍🏻

Copy link
Contributor

@pickfire pickfire left a comment

Choose a reason for hiding this comment

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

Nice, thanks for fixing the issue.

@pickfire
Copy link
Contributor

I think the check is stuck or something, I don't see the approve and run button, maybe need to create a new pull request?

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.

Picker won't scroll down when it hits the bottom
3 participants