Add TabView to Autohide Layout #1097
Conversation
Rohitth007
added
PR needs review
There was a problem hiding this comment.
@Rohitth007 This is great work 👍
This conflicts with the first commit of #1074 - and sort of needs to switch the code/style back? It'd be reasonable to extract the namespace change in that first commit and have it as another commit first in this PR (to set the module-constant precedent to build upon with TAB_WIDTH), and then have this PR's current commits - possibly including the other change from that other PR first commit, but it may be different with this PR code present?
Regarding the second refactor in #1074, as per my comment below I think that would perhaps fit with a vertical-divider refactor commit which would tidy up the inconsistent use of lineboxes? (also enum would be more descriptive than an integer when we're not seeking compatibility with urwid precisely)
neiljp
added
PR awaiting update
Rohitth007
force-pushed
the
tab-autohide
branch
2 times, most recently
from
015a706
to
a892d54
Compare
This commit moves from View constants to module constants to improve consistency with other constants. Tests updated.
This commit simplifies the code used in show_left_panel and show_right_panel to improve readability and remove unnecessary focus calls already handled by keypress. Tests updated.
This commit creates a new widget TabView for use as closed side tab in any place appropriate. This takes a single string input and renders in one character below the other by centering it vertically and horizontally. Tests added.
This commit adds a closed tab view of width 3 for each of the side panels when in the autohide layout. The initial layout when the app is started is left panel open, right panel closed. This makes use of the TabView class and changes show_left_panel and show_right_panel accordingly. Tests updated.
|
@Rohitth007 You made a few extra changes compared to the previous version so that needed a little more double-checking, but it looks good 👍 Thanks for tidying this up after the first version, this will make autohide a lot less cryptic - I'm merging it shortly 🎉 Other than a very minor test combination, the only change I made was to use an application-focused name for the symbols. We could definitely explore refinements to the current design, but that's easier and not as critical as having something present to work with :) We could maybe make them more literally like "tabs" visually, and possibly we don't need "arrows" as such (the current ones also seem offset slightly), and maybe we want to keep the tabs present even when expanded, to show the connection? However, we have a great start point to build from :) |
neiljp
removed
the
PR awaiting update
What does this PR do?
panels when in the autohide layout. The initial layout when the app
is started is left panel open right panel closed. This improves the UX and also makes it easier
for first time users to figure out what is happening.
#zulip-terminal > Autohide Tabs
Tested?
Commit flow
and the second commit uses it in autohide layout.
Notes & Questions
Interactions
Visual changes
OnStartup
BothPanelsClosed