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

Add stacked tiles feature to tiling mode #481

Merged
merged 23 commits into from
Sep 25, 2020
Merged

Conversation

mmstick
Copy link
Member

@mmstick mmstick commented Aug 5, 2020

Merge with pop-os/session#22

Fixes #538
Fixes #522
Closes #150
Closes #488

@mmstick mmstick mentioned this pull request Aug 5, 2020
@mmstick mmstick force-pushed the stacking_focal branch 4 times, most recently from 2285ccf to d8b649a Compare August 6, 2020 03:01
@mmstick mmstick force-pushed the stacking_focal branch 5 times, most recently from 6df2541 to 2ff0911 Compare August 12, 2020 14:32
@mmstick mmstick requested review from a team August 12, 2020 14:33
Copy link
Member

@jacobgkau jacobgkau left a comment

Choose a reason for hiding this comment

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

I've gotten into a state where I have a ghost window after attempting to swap a regular window with a window within a stack:

shell-pr481-screenshot1

It still takes up space in the tree, I can move it around and resize it. When I swapped it with a stacked window again, it re-appeared, but then another window in the same position disappeared when I swapped another time. (The first time it happened was with Geary, the second time was with Firefox.) This is with 2ff0911.

@mmstick
Copy link
Member Author

mmstick commented Aug 13, 2020

@jacobgkau Is there a good way to replicate this condition?

@jacobgkau
Copy link
Member

jacobgkau commented Aug 13, 2020

@jacobgkau Is there a good way to replicate this condition?

I haven't found an exact step-by-step process to reproduce it yet, but I did just get it to happen again on a fresh boot. I basically just opened up windows, created a stack, and started moving/swapping things around until it occurred. This time, I only had two displays attached.

shell-pr481-screenshot2-v3

(The red box is covering up Geary because my email is visible.) Pop Shop is what I'm focused on in the screenshot (and is hidden.) It took a little while to occur. I was initially trying to get it to happen with only a single display, I ended up attaching the second display because it didn't seem like I was getting anywhere with a single display (not sure if the second display is actually related, though.)

I'm trying to address some of the other shell PRs that are open, but when there's time, I can start a screen recording just after booting and try to get a recording of this happening if all else fails (that would include the exact steps taken to get to that point, since it would be difficult to list out the large number of very specific movements in a readable format.)

@mmstick
Copy link
Member Author

mmstick commented Aug 13, 2020

I'm working on it. I see that it happens because the actor gets hidden. Fix releasing soon.

@jmmaranan
Copy link
Contributor

@mmstick - can easily reproduce it by toggling the opened windows (minimize/restore) in Dash to Dock. But don't think it is Dash to Dock. It does not need to be stacked previously. Am on stacking branch

image

@WatchMkr
Copy link
Contributor

This is looking great. A few things:

  1. Unable to resize a stack width with shortcuts or the mouse
  2. When a stack is above another window that's not in a stack, I can only increase the stack height not decrease and the active hint remains at the original window location
  3. When moving windows outside a stack, they often move to a location I don't expect. Left/right results in the window moving above or below the stack instead of to the left. Up/down moves the window to the left or right of the stack. It seems the logic is simply inverted.

@WatchMkr
Copy link
Contributor

After leaving my laptop locked overnight, windows were all on one of two external displays and covering each other and the stacks. I'll try to get an easier way to reproduce this. Suspend and resume worked well.

@jacobgkau
Copy link
Member

I'm still seeing some inpredictability when swapping windows in and out of stacks. Take a look at this example: https://www.dropbox.com/s/eyfugswr7oh2gnk/shell-pr481-screencast1.webm?dl=0

Pop Shop was not selected or visible. Why did it come out of the stack instead of Firefox? Is this intentional because it's assumed I wouldn't swap the two windows that I'd just previously swapped?

@mmstick mmstick force-pushed the stacking_focal branch 2 times, most recently from 686aa6e to a7ea7fc Compare August 25, 2020 05:45
@mmstick mmstick requested a review from jacobgkau August 25, 2020 14:43
@mmstick mmstick force-pushed the stacking_focal branch 2 times, most recently from b7e1abb to b504920 Compare August 25, 2020 15:04
@mmstick
Copy link
Member Author

mmstick commented Aug 25, 2020

All seems to be well now. The issue with stack swapping was that a focus selection sometimes selected a hidden window in the stack.

@jacobgkau
Copy link
Member

@mmstick I'm now seeing an issue with the active hint appearing over a stack instead of the focused window:

shell-pr481-screenshot3

In this example, Chromium is focused, but the active hint is appearing over the stack. If I focus on Gedit (next to the stack), the active hint appears over Gedit correctly, but moving down to either Chromium or the terminal results in the active hint going back to the stack. Here is a screen recording to better demonstrate: https://www.dropbox.com/s/dgtodbyuugwtzc2/shell-pr481-screencast2.webm?dl=0

This is with 8cb674c.

@mmstick
Copy link
Member Author

mmstick commented Aug 25, 2020

@jacobgkau Is that the only issue you see? Trying to replicate it right now

@jacobgkau
Copy link
Member

Is that the only issue you see?

Yes, just completed the full test and I did not run into any other issues (that were not already present before this PR.) shell-pr481.txt

@jacobgkau
Copy link
Member

jacobgkau commented Sep 24, 2020

The tab bar is sometimes covering up Firefox if it was stacked and then full-screened:

shell-pr481-screenshot6

It seems to not happen the first time it's full-screened after being selected in the stack, but happens subsequent times that it's full-screened until I navigate to another item in the stack and back. Happens with both the F11 full-screen and when full-screening a video. Also affects Chromium (and presumably any full-screenable apps.)

@jmmaranan
Copy link
Contributor

@jacobgkau - it is happening in 3b21d27. So it is not introduced in the 2 recent commits. I will check

@jacobgkau
Copy link
Member

jacobgkau commented Sep 24, 2020

Another thing I noticed, when moving a window across displays, I'm able to make it cover up the rest of the windows on the second display if I try moving it past the edge of the display. https://www.dropbox.com/s/fp1z1p5tgmzi99i/shell-pr481-screencast14.webm?dl=0

Here's another example with less windows open and no stacks: https://www.dropbox.com/s/ayylv7ucaei0x88/shell-pr481-screencast15.webm?dl=0

It's almost like the window is being floated, except it's not, I'm getting to that point just by pressing the arrow keys in window management mode (one too many times), and I'm able to move the window back into a proper position with more arrow keys. Sometimes, this only happens once and the window doesn't cover up other windows again once it's been placed down, but other times, I'm able to move it back and forth between the larger state and the correct state.

I tried reproducing this one in master, and I did not see it happen there.

@jmmaranan
Copy link
Contributor

@mmstick - added fix for hiding tabs on full screen.

@jmmaranan
Copy link
Contributor

@jacobgkau - for the last one, I'd probably leave that to @mmstick

@mmstick
Copy link
Member Author

mmstick commented Sep 24, 2020

I'm looking into that last one

@mmstick
Copy link
Member Author

mmstick commented Sep 24, 2020

Fixed the tree creation on trying to move a window off a display with management mode movements

@jacobgkau
Copy link
Member

jacobgkau commented Sep 24, 2020

(Confirmed that the tabs on full-screened apps and overlapping windows when moving off the side of the display are both fixed.)

I've got a general observation and an issue that seems related. Right now, turning auto-tiling off and back on sometimes moves all of the windows back to the primary display. I'm used to seeing windows get moved around, but I don't recall ever seeing the secondary display get cleared out like this. https://www.dropbox.com/s/1t34k9aiu8w5rhx/shell-pr481-screencast16.webm?dl=0

The issue is that if I have windows on more than one workspace and I turn auto-tiling off and back on, the windows on the non-visible workspace get restricted to a certain area. At the same time, the shell is reserving space for the non-visible windows on visible workspace. https://www.dropbox.com/s/aowknacmck8cme4/shell-pr481-screencast17.webm?dl=0

If I move windows around to create more ghost window space, the windows on the other workspace are resized to fill that empty space. https://www.dropbox.com/s/ry849qz3mi5xfow/shell-pr481-screencast18.webm?dl=0

@mmstick
Copy link
Member Author

mmstick commented Sep 24, 2020

@jacobgkau Does that still happen with 4fc8df1f50c28cddbac31a88dd3647eb9bd6452c?

@jacobgkau
Copy link
Member

Does that still happen with 4fc8df1f50c28cddbac31a88dd3647eb9bd6452c?

No, after reverting to 4fc8df1 I am not seeing either of those behaviors.

@mmstick
Copy link
Member Author

mmstick commented Sep 24, 2020

Okay, I amended that last commit so it won't do that anymore

@mmstick
Copy link
Member Author

mmstick commented Sep 24, 2020

Fixed the active hint border still being around what used to be a stack after disabling tiling

jacobgkau
jacobgkau previously approved these changes Sep 24, 2020
Copy link
Member

@jacobgkau jacobgkau left a comment

Choose a reason for hiding this comment

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

All regression testing is passing and I am not seeing any bugs: shell-pr481-run15.txt

One final observation, it's sometimes unintuitive which window is focused upon switching focus to a stack just after swapping a window in the stack with a window outside of the stack. In some cases, instead of the visible window (which was just swapped into the stack) being focused, the previously-selected window in the stack is focused.

Vertical example: https://www.dropbox.com/s/nsu1plsqtp3nzip/shell-pr481-screencast19.webm?dl=0

Horizontal example (which demonstrates that the order of focus matters): https://www.dropbox.com/s/otjy9fxwv1ry6h7/shell-pr481-screencast20.webm?dl=0

This doesn't seem like a showstopper, it just took me a while to understand the logic. I can review again if this is something UX thinks should change.

@jmmaranan
Copy link
Contributor

@jacobgkau - can you check make listen when you combine in the stack or close windows? I am seeing an excessive polling in the logs. And the glib timeout never expires the first time it is called. I am working on a fix now.

@jmmaranan
Copy link
Contributor

#565 - this fixes the excessive polling.

@mmstick mmstick merged commit 9286532 into master_focal Sep 25, 2020
@mmstick mmstick deleted the stacking_focal branch September 25, 2020 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants