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

Keypress Rules Refactor: Buttons #1117

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Rohitth007
Copy link
Collaborator

What does this PR do?
This is the first of the keypress rules refactors which refactors Buttons.
The idea of following keypress rules is to:

  • send key to inner-most widget.
  • handle unhandled keys from super() and return None.
  • return unhandled keys.

Tested?

  • Manually
  • Existing tests (adapted, if necessary)
  • New tests added (for any new behavior)
  • Passed linting & tests (each commit)

Commit flow

The first commit adds spaces to improve consistency between all keypress' and improve readability.
The second one does the actual keypress refactor for Buttons.

@zulipbot zulipbot added the size: XL [Automatic label added by zulipbot] label Aug 10, 2021
@preetmishra preetmishra added the PR needs review PR requires feedback to proceed label Aug 10, 2021
@Rohitth007 Rohitth007 force-pushed the keypress-rules-1 branch 3 times, most recently from 9fb1700 to 5a1ebb3 Compare August 14, 2021 04:29
@Rohitth007 Rohitth007 changed the title Keypress Rules Refactor Keypress Rules Refactor: Buttons Aug 14, 2021
This commit adds spaces after each keypress conditional to improve
readability and consistency across all keypress'.
This commit refactors keypress to follow urwid rules, i.e.,
* send key to inner-most widget.
* handle unhandled keys from super() and return None.
* return unhandled keys.

Intercepted keys:
* " " (space) is not sent to super() in TopButton as it
an ACTIVATE key and we already have ENTER.

Tests updated.
@zulipbot
Copy link
Member

Heads up @Rohitth007, 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has conflicts PR needs review PR requires feedback to proceed size: XL [Automatic label added by zulipbot]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants