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

👀 Disable scrollbar when not sorting by name #522

Merged
merged 3 commits into from
Dec 5, 2023

Conversation

Maxr1998
Copy link
Collaborator

@Maxr1998 Maxr1998 commented Nov 1, 2023

Apologies for the messy PR, there was apparently a lot of incorrectly formatted code as well which Android Studio auto-formatted for me.

@Chaphasilor
Copy link
Collaborator

I can't figure out what the actual change is. Which scrollbar is being disabled and why? 🤔

@Maxr1998
Copy link
Collaborator Author

Maxr1998 commented Nov 2, 2023

Apologies, the PR is definitely messy. I should've redone it with two separate commits.

@Chaphasilor
Copy link
Collaborator

Okay, now I get it. Makes sense 👍🏻
If you want me to I could just force-push the branch and split it into two commits before merging it...

@Maxr1998
Copy link
Collaborator Author

Did just that myself now 😄
I actually wanted to do that since I made my last comment, but forgot about it. Thanks for giving me another poke.

@Chaphasilor
Copy link
Collaborator

Awesome. I think we can ignore the failing build for now, that problem needs to be solved somewhere else...

@Maxr1998
Copy link
Collaborator Author

Maxr1998 commented Nov 19, 2023

It's probably because I rebased onto the latest main..

@Chaphasilor Chaphasilor self-assigned this Nov 28, 2023
@jmshrv
Copy link
Owner

jmshrv commented Nov 28, 2023

The GitHub Action always uses the latest stable Flutter, which might've changed something tiny that broke the build

@Chaphasilor
Copy link
Collaborator

Yeah I fixed it in a different PR and that's now merged on main.
The changes also look good, just took a look at it :)

@Chaphasilor
Copy link
Collaborator

I think this can be merged. I'll do that in a few days unless anyone sees a problems with it...

@Maxr1998
Copy link
Collaborator Author

Maxr1998 commented Nov 28, 2023

That would be great, thank you!

Oh, and if you need me to rebase in the future, just give me a heads-up and I'll do that. It's probably cleaner than creating those merge commits.

@Chaphasilor Chaphasilor merged commit eb41f3c into jmshrv:main Dec 5, 2023
2 checks passed
@Chaphasilor
Copy link
Collaborator

Merged, thanks :)
Would you be willing to create a small PR that adds a setting for turning off the scrollbar completely? 😁

@Maxr1998 Maxr1998 deleted the hide-scroller branch December 5, 2023 23:29
@Maxr1998
Copy link
Collaborator Author

Maxr1998 commented Dec 5, 2023

Probably, bit busy though at the moment. Maybe within the next few days.

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.

3 participants