-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Start of improved layering #1508
Conversation
Can we safely merge this without breaking compatibility of the 3.x releases, or should this be targeted towards a 4.0 release? I'm a bit afraid the changes in this PR become stale if this hangs around for too long. I think as long as we maintain compatibility with the public 3.x api it should be okay to merge, no? |
I think this maintains the public API fine. It will break VS Code for example but that's because I'll polish it up now as merging this will really help with pulling components into their correct spots. |
Ok I think it's ready for proper review(s) |
Still a long way to go, but it's a good start 😅Tested locally and works fine 👍 |
Works in VS Code so merging in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious what people think of this change, I haven't gone and done a polish pass on this yet but the basic structure I think we should start with when considering #1507 is there.
IKeyboardEvent
incommon
core/input/Keyboard.ts
and added some testspublic
which is the new API. This will allow us to make changes to./src/Terminal
and eventually move it intocore
. This will break any integrations that access private API (which we don't guarantee).