Fix vertical selection of neighbors

[PabloLec]

Quick summary of pull request...

• I have read the contribution guidelines
• CI Unit tests pass
• New functions/classes have consistent docstrings

What this pull request changes

This PR only modifies the _get_vertical_neighbors method for PyCUI object.
And... (sorry for that, I just noticed it) a few auto removal of useless whitespaces, sorry for the commit readability.

This is about issue #151.
I modified the method in order to:

1. Get row scope to consider (UP) from current row to 0 or (DOWN) for current row to max row.
2. Iterate through rows in that scope.
3. For each row, get row items and ignore non selectable items.
4. Sort row items by absolute column distance. In order to, e.g we are on column 8, we probably want to jump down to next item below on column 7 rather than item on column 1.
5. Populate this neighbor list, and return this list of ids just as before.

Issues fixed with this pull request

• Vertical navigation is broken #151

Optionally, what changes should join this PR

• I didn't test it but I think left and right arrow key navigation also allow to select non selectable widgets.

• By the way, noticing my auto removal of useless whitespaces, I'd like to refactor some lines in py_cui, like unnecessary nesting, casting, etc. So, not replacing explicit code by unreadable smart code but essentially shorten some small parts in order to improve readability.
  The thing is I see multiple possible lines I could refactor in the whole codebase. I could do it and make a PR of a few commits, not a monstrosity, probably a diff of around 150 lines.
  I think it would be a good idea, I'm just wondering if the timing is good or if you're working on something right now and prefer to avoid blocking merges.

[PabloLec]

I just updated RecoverPy demo GIF too. Better quality + I make a better use of PyCUI. Better showcase of PyCUI overall 😉

[voryzen]

Hey @PabloLec,
Does this need any testing to be done?

[PabloLec]

Hi! Sorry for the delay, the fix did work so no further testing needed but the same fix needs to be applied to horizontal selection though.

[voryzen]

OK, no worries @PabloLec
Just wondering if I could help.

Fixing the horizontal and vertical nav would be a game changer for me @jwlodek

[jwlodek]

@PabloLec Fixed some merge conflicts created by merging the formatting PR, but not 100% sure if the expected behavior is retained. Could you please double check? Also, if you could add some additional unit tests that account for the newly fixed vertical selection that would be great - basically just adding a new test file that tests vertical selection as it is expected to behave, in cases of successful upwards, successful downwards, unsuccessful downwards, unsuccessful upwards, and bottom/top of the UI as well.

[voryzen]

Hi @PabloLec and @jwlodek,

Good to know your still around @jwlodek

@PabloLec
it should be noted that the new navigation, does not factor in 'unselectable' widgets that separate selectable widgets.

for example:
widget_a at 1,1 (selectable),
widget_b at 1,2 or 2,1 (unselectable), and
widget_c at 1,3 or 3,1 (selectable)

when attempting to navigate using the new code, the user will not be able to move from widget_a to widget_c, using the arrow keys.

I gather this has something to do with line 1133
return neighbors[0]
in the _check_if_neighbor_exists function.

I have yet to work out how to rectify this. any thoughts @PabloLec.

I also note that the cycle widgets function does not respect unselectable widgets either, even after implementing the new navigation code. Any ideas here would be appreciated, I think I might have a way to do it, but I won't get to it, for another few days, due to study commitments. Any ideas here would be welcome. Fixed in PR #171

[voryzen]

@PabloLec I tried to fix the horizontal navigation, but it doesn't seem to work.
I will note though, that vertical navigation does respect the 'unselectable' widgets. all good there

[voryzen]

hey @PabloLec @jwlodek any updates on this?