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

Javalab: Use codemirror #39147

Merged
merged 12 commits into from Feb 22, 2021
Merged

Javalab: Use codemirror #39147

merged 12 commits into from Feb 22, 2021

Conversation

molly-moen
Copy link
Contributor

@molly-moen molly-moen commented Feb 19, 2021

Instead of using Ace as our editor in Javalab, switch to Codemirror version 6

Our main reason for the switch is codemirror 6 has been redesigned with accessibility in mind and has things such as tab navigation built in. Ace does not have a solution for tab navigation (you get stuck in the ace editor with no way out beyond us doing something hacky).

Note that this is a different version of codemirror than we use elsewhere because we want to take advantage of the new accessibility features. However the new codemirror has different package names than the old one so it should not be an issue.

We still allow tabbing in the editor, but the user can continue navigation by doing esc+tab.

The new editor looks like this:
Screen Shot 2021-02-18 at 4 23 04 PM

Links

Testing story

Tested locally. Verified we are still writing updates to redux so we can save/run code later (we do not have that functionality yet).

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

dispatchEditorChange = () => {
return tr => {
// we are overwriting the default dispatch method for codemirror,
// so we need to manually call the update method.
Copy link
Contributor

Choose a reason for hiding this comment

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

This commenting is very helpful :)

}

dispatchEditorChange = () => {
return tr => {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is tr in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tr is a codemirror transaction I'll add a comment to make it more clear

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@jmkulwik
Copy link
Contributor

Haha I'm glad I'm in an empty room, because I might have otherwise startled anyone nearby. So excited about this change!!!

@molly-moen molly-moen merged commit 3ec5db8 into staging Feb 22, 2021
@molly-moen molly-moen deleted the java-ide-codemirror branch February 22, 2021 21:38
@@ -0,0 +1,52 @@
import {
Copy link
Contributor

@jmkulwik jmkulwik Feb 22, 2021

Choose a reason for hiding this comment

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

This separation of concerns will make maintenance easy and straightforward 🙂

Copy link
Contributor

@jmkulwik jmkulwik left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@jmkulwik jmkulwik left a comment

Choose a reason for hiding this comment

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

LGTM!

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

2 participants