zulip-terminal
zulip-terminal copied to clipboard
boxes: Disable cycling using arrow keys if recipient is invalid.
This PR disables cycling from the recipient box to any other box inside the WriteBox view using arrow keys, if the recipient name is invalid. This is very similar to CYCLE_COMPOSE_FOCUS using Tab key. Tests have also been updated and amended.
This PR fixes #779.
@zee-bit Just some quick feedback:
- is there a lot of duplication here? Could we reuse parts to make this a lot simpler?
- the tests look large and complex too
- is there some way we can trigger on 'losing focus', to avoid having all these cases? (that might cover the mouse as well, as a minor side-effect)
@neiljp Regarding your feedback:
- I have made two functions(
footer_notify_invalid_stream&footer_notify_invalid_emails), they have somewhat increased code reusability. The main logic, I think, could not be simplified further if I am to proceed with the current implementation. - The tests are very much related to that of
CYCLE_COMPOSE_FOCUSas it is derived from there, but there were few places with unnecessary conditions and checks that I have removed. - I am not quite sure if there is any other way to trigger on 'loosing focus'(without all those conditions). If you or @preetmishra have some other implementation in mind, I would love to learn :). Re mouse event, I can try extending this for mouse events as well, if this implementation looks fine.
@zulipbot add "PR needs review"
I should say that it does seem to work OK :+1: I've not found a signal emitted within urwid that would solve this for us as it stands.
@zulipbot add "PR needs review" @zulipbot remove "PR awaiting update"
@zulipbot add "PR needs review" @zulipbot remove "PR awaiting update"
Heads up @zee-bit, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.