-
-
Notifications
You must be signed in to change notification settings - Fork 681
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(ui): The status bar indicates that the panes are full screen and how many hidden panes are #450
Conversation
Do you have a screenshot? Is there any ETA about the approval and merge of this PR? |
@phaazon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, I'm really happy you took this up again! :)
I left some comments. The main one is that I think this should be in the status bar instead of the tab bar. Specifically instead of the line that starts with "Tip:" whenever we're in full screen.
Looking forward to merge this feature! I know for me personally it will be super helpful. I'm in full screen a lot and sometimes forget that I am, and then open new panes and everything gets messed up :)
@imsnif Screen.Recording.2021-09-10.at.1.08.19.mov |
Thanks @takezyou, this works already great.
Now we are not in fullscreen anymore, but the status bar still shows |
@a-kenji |
@takezyou - good progress! I found another issue: it seems that the message only works if you're on the last opened tab. To reproduce:
About the visuals: I'm going to be away for a few days, so I hope @a-kenji has time and can answer questions if you have them. Otherwise I can take another look on Wednesday. |
@a-kenji |
@takezyou |
@a-kenji |
@takezyou |
@a-kenji |
@takezyou |
@a-kenji |
…e of the status-bar.
491c17f
to
a3b9338
Compare
@a-kenji |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@a-kenji - from a brief glance it looks like there's a crash with a stacktrace in (all?) tests inside the container (this is distinct from the stacktrace of the failing test itself - you can see the tail end of it in the provided snapshot), eg.: I would start by trying to replicate one test manually by SSHing into the container and doing all the steps that it does. You'll likely see the same stack trace and be able to understand what's causing it exactly. I have a bit of a busy week, so not sure I'll have time to take a closer look soon unfortunately. If everyone is really stuck though I'll try to make time in one evening or the weekend. |
Hum. Github doesn't seem to want to allow links to the build logs. Look for a line starting with "current_snapshot:" - this is a representation of what the test "sees on the screen", and in most cases there's the end of a stacktrace. |
@imsnif
and then
I will try to recreate this in a virtual machine, but I suspect this will be somewhat slow. |
No need for a virtual machine (unless I'm missing something?) On this branch, do:
|
@imsnif
I had trouble getting docker compose working on my machine, but it seems |
Allow specifying only the tab name in the `tabs` section - For example this is now possible: ``` tabs: - name: first parts: - direction: Vertical - direction: Vertical - name: second - name: third ``` For that the tab section defaults the direction to `direction::Horizontal` - Adds an error upon specifying a tab name inside the `parts` section of the tab-layout
…org#660) * feat(plugins-manifest): Add a plugins manifest to allow for more configuration of plugins * refactor(plugins-manifest): Better storage of plugin metadata in wasm_vm * fix(plugins-manifest): Inherit permissions from run configuration * refactor(plugins-manifest): Rename things for more clarity - The Plugins/Plugin structs had "Config" appended to them to clarify that they're metadata about plugins, and not the plugins themselves. - The PluginType::OncePerPane variant was renamed to be just PluginType::Pane, and the documentation clarified to explain what it is. - The "service" nomenclature was completely removed in favor of "headless". * refactor(plugins-manifest): Move security warning into start plugin * refactor(plugins-manifest): Remove hack in favor of standard method * refactor(plugins-manifest): Change display of plugin location The only time that a plugin location is displayed in Zellij is the border of the pane. Having `zellij:strider` display instead of just `strider` was a little annoying, so we're stripping out the scheme information from a locations display. * refactor(plugins-manifest): Add a little more documentation * fix(plugins-manifest): Formatting Co-authored-by: Jesse Tuchsen <not@disclosing>
@a-kenji - as discussed offline, the e2e tests using the debug build was a very good find and I'm fixing it elsewhere. I don't think this is the issue here though. If you don't have time to take a look at this, I might be able to free some time up for this on the weekend. Let me know. |
@imsnif |
@imsnif I think I found the issue for the flaky tests. |
@a-kenji that's super weird. I just checked the musl release build and it runs just fine for me. What are you doing when it's closing panes for you? I did a lot of work on the e2e tests today in the mirrored-sessions branch. It might be a pain to get them to work with this branch though (and I'm still not done - there's 1 test that often fails on the CI, but it specifically tests the mirrored sessions so you can safely ignore it). |
@imsnif
Should I push the changes I made? |
@a-kenji - what changes? |
And I changed from debug to release build, but there are still some issues. |
If it still passes once you do (all except the one test there, the mirror test, that isn't currently passing) then sure |
* feat(sessions): mirrored sessions * fix(tests): input units * style(fmt): make rustfmt happy * fix(tests): make mirrored sessions e2e test more robust * refactor(sessions): remove force attach * style(fmt): rustfmtify * docs(changelog): update change * fix(e2e): retry on all errors
…ou/zellij into takezyou-feature-full-screen-panes
@takezyou |
Implementation of #168
I plan to display it in the tab bar instead of the status bar