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

fix(tiling): Windows launching onto window that isn't focused #1171

Merged
merged 4 commits into from
Sep 14, 2021
Merged

Conversation

mmstick
Copy link
Member

@mmstick mmstick commented Sep 9, 2021

Closes #1166

@mmstick mmstick requested review from a team September 9, 2021 23:07
jackpot51
jackpot51 previously approved these changes Sep 10, 2021
@jacobgkau
Copy link
Member

The wording in #1137 seems to imply that windows should quarter-tile first. After this change, I am now seeing the "incorrect" behavior of new windows continuing down the same side of a fork instead of alternating back to quarter tile:

simplescreenrecorder-2021-09-10_09.30.41.mp4

This behavior does seem logical, but we need clarification on what the expected behavior is.

@mmstick
Copy link
Member Author

mmstick commented Sep 10, 2021

Windows should always launch onto the window that is focused, so you can expect to know where the window is tiled.

@jacobgkau
Copy link
Member

I understand, #1137 is just referring to the split happening horizontally instead of vertically.

@jackpot51
Copy link
Member

@jacobgkau this behavior in this PR is the correct behavior as far as I know. A new window should always halve the focused window, or join the focused window's stack.

@jacobgkau
Copy link
Member

I'm seeing cases across multiple displays where a new window is not joining a focused window:

Screencast.from.09-10-2021.10.15.20.AM.mp4

I believe the key to recreating this is switching between several stacked windows just before switching focus to another display and opening a new one.

jackpot51
jackpot51 previously approved these changes Sep 13, 2021
@jacobgkau
Copy link
Member

The stacking case from before is fixed. However, I'm still seeing new windows appear on the non-focused display if the mouse cursor is on the non-focused display. (Basically, the cursor seems to override focus for choosing which display to use.)

simplescreenrecorder-2021-09-14_08.50.12.mp4

Is this how it's supposed to work? It seems incorrect based on the description of "should always launch onto the window that is focused."

@mmstick
Copy link
Member Author

mmstick commented Sep 14, 2021

It should be the active window that matters more than the mouse position. Will look

@jacobgkau
Copy link
Member

That commit fixed the remaining issue with window opening in tiled mode. Windows now always open on the focused window/stack, regardless of cursor position.

However, it introduced a change of behavior when entering tiling mode; all windows now tile onto the primary display. Before:

simplescreenrecorder-2021-09-14_10.19.41.mp4

After:

simplescreenrecorder-2021-09-14_10.20.12.mp4

Can existing windows be tiled onto their current display when entering tiling mode, without changing the behavior for opening new windows?

@mmstick
Copy link
Member Author

mmstick commented Sep 14, 2021

Sure. Done.

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.

Everything looks good now. Regression testing passed:
shell-pr1171-hirsute.txt
shell-pr1171-focal.txt

@mmstick mmstick merged commit e3c1d35 into master Sep 14, 2021
@mmstick mmstick deleted the focus branch September 14, 2021 18:07
LinuxHeki pushed a commit to LinuxHeki/shell that referenced this pull request Oct 3, 2021
@LinuxHeki LinuxHeki mentioned this pull request Oct 3, 2021
LinuxHeki pushed a commit to LinuxHeki/shell that referenced this pull request Oct 14, 2021
jacobgkau pushed a commit to LinuxHeki/shell that referenced this pull request Oct 14, 2021
Undo fix(tiling) pop-os#1171

fix(tiling): Windows launching onto window that isn't focused

Corrected random spaces in code
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.

Incorrect fork order
3 participants