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

Context Menu doesn't work when container has transform: translate #1911

Closed
zxlin opened this issue Jan 26, 2019 · 5 comments · Fixed by #1912
Closed

Context Menu doesn't work when container has transform: translate #1911

zxlin opened this issue Jan 26, 2019 · 5 comments · Fixed by #1912

Comments

@zxlin
Copy link

zxlin commented Jan 26, 2019

Details

  • Browser and browser version: Chrome 71.0.3578.98
  • OS version: Windows 10 & MacOS Mojave
  • xterm.js version: v3.10.1

Steps to reproduce

  1. Visit https://o9xjjwo47y.codesandbox.io/
  2. Right click anywhere inside the black xterm terminal
  3. See that it opens up the canvas context menu instead of the textarea context menu

Related: #1810

Example code snippet

Also here: https://codesandbox.io/s/o9xjjwo47y

import React from "react";
import ReactDOM from "react-dom";
import Draggable from "react-draggable";
import { Terminal } from "xterm";
import * as fit from "xterm/lib/addons/fit/fit";
import "xterm/dist/xterm.css";

import "./styles.css";

Terminal.applyAddon(fit);

class App extends React.PureComponent {
  constructor(props) {
    super(props);

    this.state = {
      x: 0,
      y: 0
    };
  }
  componentDidMount() {
    const term = new Terminal();
    term.open(document.getElementById("xterm"));
    term.fit();
    term.write("Hello from \x1B[1;3;31mxterm.js\x1B[0m $ ");
  }

  render() {
    const { x, y } = this.state;
    return (
      <div className="App">
        <h1>Xterm.js Right Click Bug Example</h1>
        <p>
          Right click anywhere inside the black terminal and you will see that
          the canvas context menu shows up instead of the textarea context menu
        </p>
        <p>
          If you open dev-tools and remove the "transform: translate" style from
          the parent container div, then the correct context menu shows up
        </p>
        <Draggable
          position={{ x, y }}
          onDrag={(e, data) => this.setState({ x: data.x, y: data.y })}
        >
          <div
            id="parent-container"
            style={{
              width: 500,
              height: 500,
              padding: "1em",
              background: "#333"
            }}
          >
            <div id="xterm" />
          </div>
        </Draggable>
      </div>
    );
  }
}

const rootElement = document.getElementById("root");
ReactDOM.render(<App />, rootElement);

@Tyriar
Copy link
Member

Tyriar commented Jan 26, 2019

Here are the lines that likely need to change to fix this:

textarea.style.left = (ev.clientX - 10) + 'px';
textarea.style.top = (ev.clientY - 10) + 'px';

@zxlin
Copy link
Author

zxlin commented Jan 26, 2019

@Tyriar thanks for being so attentive to this, any initial thoughts on what's causing this?

I don't know enough about CSS to usefully diagnose this, but if you give me some thoughts I can try to test some stuff out to help hone in on the root cause.

@Tyriar
Copy link
Member

Tyriar commented Jan 26, 2019

@zxlin ev.clientX and ev.clientY are probably relative to the wrong element, that would need to be tweaked to work in both cases.

@mofux
Copy link
Contributor

mofux commented Jan 27, 2019

Very strange, looks like a browser bug to me. My guess would be that the textarea does not yet have focus at the very moment the context menu is opened, but it only seems to happen if a translate is active on the element. Maybe the browser rendering pipeline differs while a transformation is active, which causes this issue. It also happens on Firefox...

Is there any specific reason you use translate? CSS transformations run on the GPU and don't force a relayout, which is why they are so fast to render, but it's usually a bad idea to use them for layouting.

@zxlin
Copy link
Author

zxlin commented Jan 27, 2019

@mofux We have xterm inside a draggable container to allow the user to lay out their work space to their personal preference. That's simply the lib's way of implementing draggable.

One thing of note here is that the transform style doesn't need to be on the parent element of xterm to cause this issue, if any of xterm's ancestor elements has the transform style, this bug occurs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants