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

feat: add focus attribute in layout #958

Merged
merged 11 commits into from
Jan 31, 2022
Merged

feat: add focus attribute in layout #958

merged 11 commits into from
Jan 31, 2022

Conversation

jaeheonji
Copy link
Member

@jaeheonji jaeheonji commented Dec 20, 2021

This PR resolve #852 , resolve #942

Description

The focus attribute is added to the layout like this:

---
template:
  ~
tabs:
- direction: Vertical
- direction: Vertical
  focus: true # this
  parts:
    - direction: Vertical
      split_size:
        Percent: 50
    - direction: Vertical
      focus: true # this
      split_size:
        Percent: 50

First, each TabLayout has a focus attribute. The focus value of the top-level TabLayout determines "which tab to focus on."
The sub-level TabLayout refers to the pane of each tab, it determines "which pane to focus on".

If focus attribute is duplicated, tab or pane with the first declared value is focused.

Also, to keep the state of the last focused pane within each tab, the Tab struct has a last_active_pane_id variable.
This saves the state of the focused pane from the layout, and at the same time preserves the state of the previous pane when the tab is moved. (Previously, it was initialized to the first (Index 0) pane)

I declared the last_active_pane_id variable as an Option in case there was any problem with the system. If it's no problem, I will remove the Option type.

Other thing..

To focus the tab, internally call GoToTab Instruction after all layouts are loaded.
When zellij starts with multiple tabs layout, it currently has the following process.

newTab -> render -> newTab -> render -> newTab -> render ... -> GoToTab -> render

Of course, this is not a big problem but,

newTab -> newTab -> newTab -> ... -> GoToTab -> render

It might be a good idea to render only once, only when the program is started. Of course this is a different issue.

EDIT: I will update the test-related errors when this feature is determined to be ok.

@imsnif imsnif requested a review from a-kenji December 29, 2021 15:31
@djpate
Copy link
Contributor

djpate commented Jan 5, 2022

That's exactly what I need. +1

@djpate
Copy link
Contributor

djpate commented Jan 17, 2022

If this was passing specs, would you consider merging this? @imsnif @tlinford

@imsnif
Copy link
Member

imsnif commented Jan 17, 2022

If it passes CI and @jaeheonji thinks it's good, I'm happy.

@jaeheonji
Copy link
Member Author

@djpate Thanks for your interest!

@imsnif The original concept for this feature was presented by @a-kenji, so I respect his opinion and await a review.
However, since he was already agreed on the overall spec (although review has not been done yet), I'll merge this features after CI and strict testing.

As an aside, I'm currently busy until this week, so please be patient if the work is a little late 🙏

@imsnif
Copy link
Member

imsnif commented Jan 18, 2022

@jaeheonji - there is no rush! Please take your time.

@jaeheonji jaeheonji marked this pull request as draft January 24, 2022 06:06
Copy link
Contributor

@a-kenji a-kenji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry that it took this long.
This is awesome!
LGTM!

zellij-server/src/pty.rs Outdated Show resolved Hide resolved
zellij-server/src/pty.rs Show resolved Hide resolved
@jaeheonji jaeheonji marked this pull request as ready for review January 31, 2022 16:52
@jaeheonji
Copy link
Member Author

History Note:

I did not fully implement the function I initially expected.

In particular, the ability to save the state of the focused Pane in each Tab is not implemented at this time due to several problems.

(Because I found a conflict with the multiple user function and an unexpected bug internally. This should be managed as a separate issue.)

So, I only implement focusing on specific Tab and Pane through Layout.

@jaeheonji jaeheonji merged commit 1d2e303 into zellij-org:main Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: keep state of focusd pane in each tabs Feature: add focus attribute to the layout
4 participants