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 overlay when resizing panels to prevent panes stealing focus #19862

Merged
merged 7 commits into from Mar 31, 2020

Conversation

Aerijo
Copy link
Member

@Aerijo Aerijo commented Sep 1, 2019

Requirements for Contributing a Bug Fix

Identify the Bug

#20216

Description of the Change

Adds a div across the whole window when resizing panels.

This div keeps the context on Atom, even when a panel might normally grab it (e.g., if it were an iframe or webview). This is relevant for packages that provide this kind of panel, such as ones using the default Mozilla PDF viewer. Adding an overlay is already done by the Dock class.

Alternate Designs

I considered if it could be fixed by the package making the iframe in the first place. However, anything that could be done (if at all) would just be a hack, and wouldn't benefit any other packages that use iframes and face the same issue. This fixes the issue at its root.

Possible Drawbacks

My biggest concern is somehow not cleaning up the div after resizing stops. The Dock class already makes a similar overlay, and I am not aware of any issues with it. The method here is somewhat different, but should work so long as resizeStopped is called when resizing has stopped.

Verification Process

  1. Have the following in your init.js file
atom.workspace.addOpener(uri => {
  if (uri === "frame://example") {
    const frame = document.createElement("iframe");
    frame.setAttribute("style", "width:100%;height:100%");

    return {
      getTitle: () => "iframe",
      element: frame,
    };
  }
})

atom.workspace.addOpener(uri => {
  if (uri === "editor://example") {
    return atom.workspace.buildTextEditor();
  }
})

atom.workspace.open("frame://example");
atom.workspace.open("editor://example");
  1. Move the TextEditor panel so that both are visible side by side
  2. Resize the panels by dragging the resize element towards the TextEditor. Both with and without this PR should have no issue dragging towards the TextEditor.
  3. Resize the panels by dragging the resize element towards the iframe. Without this PR, it should only move if you drag slowly. If you drag quickly over the iframe, the resize will stop until you move off the iframe. With this PR, it works at all speeds.

Additionally:

  1. When resizing, tab out to a different application
  2. Move and release the mouse anywhere.
  3. Look back at the Atom window. It should have moved the divider where you released the mouse (this is the same as with resizing a dock).

Notes:

  • Programmatic resize methods should not be affected. This div is only created when clicking down on the resize element.
  • Double clicking the resize element to centre the division was broken by the initial attempt. The div was being made over the resize element, so a double click event was not detected. I have given the resize element a z-index of 11, and the overlay a z-index of 4. This matches the values used for the dock resize & overlay.

The following demonstrate the difference. In the first one, the mouse was not released. In the second, the mouse was released and double clicked to cause the panels to balance.
Old behaviour:
noverlay

New behaviour:
overlay

Release Notes

  • Add overlay when resizing panels to prevent panes stealing focus

@rsese
Copy link
Member

rsese commented Sep 12, 2019

Just wanted to double check @Aerijo, I saw the TODO in the PR body and peeking at the changed didn't see any tests (or is this behavior not easily testable?), you're still working on this yes? Or is it all set?

@Aerijo
Copy link
Member Author

Aerijo commented Sep 13, 2019

@rsese The TODO is for a corresponding issue, which I haven't actually made. As for tests, I don't know if it is testable, but I'll have a look and see if the Dock resize is tested. But the behaviour works as intended.

Copy link

@alissonpelizaro alissonpelizaro left a comment

No problem

@Aerijo Aerijo mentioned this pull request Dec 7, 2019
1 task
@Aerijo
Copy link
Member Author

Aerijo commented Dec 7, 2019

@rsese Updated the PR with an issue, and checked the specs for if the Dock overlay is tested. I couldn't find any such tests in spec/dock-spec.js.

@ashthespy
Copy link

ashthespy commented Mar 12, 2020

Just a gentle nudge - this would be quite nice to have! :=)

@darangi
Copy link
Contributor

darangi commented Mar 31, 2020

Thanks @Aerijo

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.

None yet

5 participants