petereon / beaupy

A Python library of interactive CLI elements you have been looking for
https://petereon.github.io/beaupy/
MIT License
181 stars 13 forks source link

Better "select" & "select_multiple" Scrolling. #43

Closed therealOri closed 1 year ago

therealOri commented 1 year ago

Updated:


Left alone:



Why?:

When you start to have pretty BIG lists, say 20 options for example...it will take some time to get all the way back to the top of the list of options. Same for when you want to get to the bottom of the list. I would have to tap the down arrow key like 19 times to get to the bottom. I can now just press up 1 time and BAM, bottom of the list. (Same for being at the bottom and getting back to the top)




Tested with:

Successfully.

  • Python Version: v3.10
  • OS: Manjaro Linux x86_64
  • Kernel: 5.15.85-1-MANJARO
therealOri commented 1 year ago

I am unsure as to why poetry run poe test would result in 3 failures. Maybe it's an issue with the tests themself expecting a different result?

2023-01-04_10-01 2023-01-04_10-01_1 2023-01-04_10-02 2023-01-04_10-02_1



I took a look at the steps that are automated and when I do them myself (press the same keys/inputs as the test does), I get exactly what I expected and want. Maybe I don't understand the tests. idk.

petereon commented 1 year ago

Hey @therealOri , thanks for the contribution. You are right! It is tests that are not adjusted to new behavior. Tests are also in dire need of rework as testing of the console output makes them heavily implementation dependent and most are pretty much intertwined at the moment which makes adding new functionality and changing existing one a bit of a a hassle.

Furthermore, I believe there are instances of tests where I was testing exactly that it does not "overflow". This might be causing issues now that it does.

I am currently pretty pressed for time but I can have a proper look on Friday I think.

therealOri commented 1 year ago

Okay!

therealOri commented 1 year ago

The patch I have applied to therealOri/beaupy that you have requested seems to have reverted _beaupy.py back to it's original state before my changes.

I have updated it again in my repository. So just make sure that when you decide to merge code, that what's being merged is what is supposed to be merged.

petereon commented 1 year ago

The patch I have applied to therealOri/beaupy that you have requested seems to have reverted _beaupy.py back to it's original state before my changes.

I have updated it again in my repository. So just make sure that when you decide to merge code, that what's being merged is what is supposed to be merged.

What do you mean reverted? Granted, the if blocks are gone, because I have refactored the changes in the _beaupy.py to use modulo instead of conditionals.

therealOri commented 1 year ago

The patch I have applied to therealOri/beaupy that you have requested seems to have reverted _beaupy.py back to it's original state before my changes. I have updated it again in my repository. So just make sure that when you decide to merge code, that what's being merged is what is supposed to be merged.

What do you mean reverted? Granted, the if blocks are gone, because I have refactored the changes in the _beaupy.py to use modulo instead of conditionals.

Ohh, Okay.

petereon commented 1 year ago

Sorry if I wasn't awfully transparent about that. But for me it seems to work, whenever it overflows the modulo bumps it to the other end of acceptable range of indexes.

therealOri commented 1 year ago

Sorry if I wasn't awfully transparent about that. But for me it seems to work, whenever it overflows the modulo bumps it to the other end of acceptable range of indexes.

It does work for me as well. And it's alright, I wasn't reading closely enough xD.

petereon commented 1 year ago

Sorry if I wasn't awfully transparent about that. But for me it seems to work, whenever it overflows the modulo bumps it to the other end of acceptable range of indexes.

It does work for me as well. And it's alright, I wasn't reading closely enough xD.

Perfect, I'll run some manual testing on Windows, hopefully get the lint action to pass and then I'll merge and release! Thanks for contribution once again!