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

[WIP] Pull mouse mode handling into its own module #1608

Closed
wants to merge 1 commit into from

Conversation

bmf-ribeiro
Copy link
Contributor

Fixes #1504

Work in progress, I have a small question directly in the code...

Feedback is welcome 😄

private _vt300Mouse: boolean; // This is unstable and never set

constructor(
private _terminal: any
Copy link
Contributor Author

Choose a reason for hiding this comment

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

How should I go about typing this? Should I add the missing properties to the IInputHandlingTerminal, ITerminal, or should I create a new interface?

Copy link
Member

Choose a reason for hiding this comment

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

The ideal situation is that MouseEventsHandler does not depend on Terminal at all, instead it only communicates back via events, this would help break the circular dependency.

Also eventually we should pull all the mouse-related properties of Terminal into MouseEventsHandler, and move the non-DOM dependent parts into ./core/input/Mouse.ts, (like ./core/input/), and the DOM dependent parts to ./ui/Mouse.ts(or./ui/UserInterface`). This direction is explained in more detail in #1507

@Tyriar Tyriar added the work-in-progress Do not merge label Sep 1, 2018
@Tyriar
Copy link
Member

Tyriar commented Nov 18, 2018

Closing this off as stale, feel free to open a new PR after rebasing if you want to continue no this 😃

@Tyriar Tyriar closed this Nov 18, 2018
@bmf-ribeiro bmf-ribeiro deleted the pull-mouse-handler branch November 27, 2019 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work-in-progress Do not merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants