Panel refactor #1072
Panel refactor #1072
Conversation
Rohitth007
force-pushed
the
panel-refactor
branch
from
7761985 to
f096a17
Compare
Rohitth007
added
PR needs review
This commit moves `enter_editor_mode` into the Stream/Topic/ RightColumnView's keypress so that View is left to handle only focus and autohide features. This helps with refactoring Panel later. Tests updated.
This commit: * replaces user_view with right_column to make it similar to left_column.keypress in SEARCH_STREAMS/TOPICS * returns middle_column.keypress(), left_column.keypress() and right_column.keypress() to directly return handled/unhandled keys.
To improve readability in `__init__` method of StreaView and TopicView, related variables were grouped together using comments.
Rohitth007
force-pushed
the
panel-refactor
branch
from
f096a17 to
a5497bd
Compare
|
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 |
There was a problem hiding this comment.
@Rohitth007 I've just resolved a conflict from the test-typing changes and merged the first commit - it's a very nice tidy, moving the action closer to the data 👍
(also avoiding that conditional just by knowing which state we're in!)
For the second commit, there's renaming and the moving to a direct return of the key - is the latter intended to help towards the keypress-flow restructuring? There's no mention of that, but the style isn't mentioned with that (or anything else) in mind, so I wasn't clear.
I've not resolved the conflicts in the second commit onwards; it looks like there still may be a need to improve the patching in the new version like in the old one, now that we're checking the types? (@prah23 ?)
| @@ -292,7 +290,7 @@ def test_keypress_autohide_users(self, view, mocker, autohide, key, widget_size) | |||
|
|
|||
| view.keypress(size, key) | |||
|
|
|||
| view.users_view.keypress.assert_called_once_with(size, key) | |||
| view.right_panel.keypress.assert_called_once_with(size, key) | |||
There was a problem hiding this comment.
Replacing users_view with right_panel seems good naming here, though the commit text doesn't indicate the same name?
There was a problem hiding this comment.
Oh, I change that right away.
neiljp
added
PR awaiting update
|
@neiljp Re second commit, it definitely helps with the keypress refactoring by allowing the return of unhandled keys back to urwid in the end but it doesn't directly affect the functionality in that PR as this is the outer most widget. |
What does this PR do?
Currently only contains prep commits that help with both Panel refactor and keypress refactor.
Tested?
Commit flow
Notes & Questions
It would be useful to merge the first two.
Interactions