nteract / play

The code base for the nteract Play app
https://play.nteract.io/
BSD 3-Clause "New" or "Revised" License
14 stars 19 forks source link

Allow pluggable editor surfaces for play #19

Open yuvipanda opened 6 years ago

yuvipanda commented 6 years ago

I'd love to be able to configure using Monaco or even just a TextArea for use as input with Play. This is important for accessibility reasons - Monaco is far more accessible than CodeMirror, and even Textarea is sometimes desirable.

@rgbkrk showed me how to do this, and it doesn't look too difficult.

rgbkrk commented 6 years ago

For posterity, it takes roughly this:

diff --git a/applications/play/components/Main.js b/applications/play/components/Main.js
index c1811e11..370c9761 100644
--- a/applications/play/components/Main.js
+++ b/applications/play/components/Main.js
@@ -179,8 +179,16 @@ class Main extends React.Component<*, *> {
             gitref={gitrefValue}
           />
         ) : null}
-
         <div className="play-editor">
+          <textarea
+            value={sourceValue}
+            onChange={event => {
+              this.handleEditorChange(event.target.value);
+            }}
+          />
+        </div>
+
+        {/* <div className="play-editor">
           <CodeMirrorEditor
             // TODO: these should have defaultProps on the codemirror editor
             cellFocused
@@ -221,7 +229,7 @@ class Main extends React.Component<*, *> {
             value={sourceValue}
             onChange={this.handleEditorChange}
           />
-        </div>
+        </div> */}

         <div className="play-outputs">
           <Outputs>
@@ -240,6 +248,16 @@ class Main extends React.Component<*, *> {
           --header-height: 42px;
           --editor-width: 52%;

+          textarea {
+            width: 100%;
+            box-sizing: border-box;
+            font-family: "Source Code Pro", monospace;
+            font-size: 12px;
+            line-height: 22px;
+            padding: 10px;
+            border-top: 0px;
+          }
+
           header {
             display: flex;
             justify-content: space-between;

and you'll have a working textarea that's "hooked up" to the kernel.

screen shot 2018-03-29 at 4 53 29 pm

yuvipanda commented 6 years ago

@rgbkrk would you like me to just switch this to monaco, or make it an option of some form between monaco / codemirror / textarea?

yuvipanda commented 6 years ago

I spent a bunch of time trying to get https://github.com/superRaytin/react-monaco-editor to work.

Currently stymied by:

ModuleParseError: Module parse failed: Unexpected token (159:12)
You may need an appropriate loader to handle this file type.
|   }
| 
|   assignRef = (component) => {
|     this.containerElement = component;
|   }
    at doBuild (/home/yuvipanda/code/nteract/applications/play/node_modules/webpack/lib/NormalModule.js:303:19)
    at runLoaders (/home/yuvipanda/code/nteract/applications/play/node_modules/webpack/lib/NormalModule.js:209:11)
    at /home/yuvipanda/code/nteract/applications/play/node_modules/loader-runner/lib/LoaderRunner.js:370:3
    at iterateNormalLoaders (/home/yuvipanda/code/nteract/applications/play/node_modules/loader-runner/lib/LoaderRunner.js:211:10)
    at Array.<anonymous> (/home/yuvipanda/code/nteract/applications/play/node_modules/loader-runner/lib/LoaderRunner.js:202:4)
    at Storage.finished (/home/yuvipanda/code/nteract/applications/play/node_modules/enhanced-resolve/lib/CachedInputFileSystem.js:40:15)
    at /home/yuvipanda/code/nteract/applications/play/node_modules/enhanced-resolve/lib/CachedInputFileSystem.js:77:9
    at /home/yuvipanda/code/nteract/applications/play/node_modules/graceful-fs/graceful-fs.js:78:16
    at FSReqWrap.readFileAfterClose [as oncomplete] (fs.js:511:3)
ModuleParseError: Module parse failed: Unexpected token (159:12)
You may need an appropriate loader to handle this file type.
|   }
| 
|   assignRef = (component) => {
|     this.containerElement = component;
|   }
    at doBuild (/home/yuvipanda/code/nteract/applications/play/node_modules/webpack/lib/NormalModule.js:303:19)
    at runLoaders (/home/yuvipanda/code/nteract/applications/play/node_modules/webpack/lib/NormalModule.js:209:11)
    at /home/yuvipanda/code/nteract/applications/play/node_modules/loader-runner/lib/LoaderRunner.js:370:3
    at iterateNormalLoaders (/home/yuvipanda/code/nteract/applications/play/node_modules/loader-runner/lib/LoaderRunner.js:211:10)
    at Array.<anonymous> (/home/yuvipanda/code/nteract/applications/play/node_modules/loader-runner/lib/LoaderRunner.js:202:4)
    at Storage.finished (/home/yuvipanda/code/nteract/applications/play/node_modules/enhanced-resolve/lib/CachedInputFileSystem.js:40:15)
    at /home/yuvipanda/code/nteract/applications/play/node_modules/enhanced-resolve/lib/CachedInputFileSystem.js:77:9
    at /home/yuvipanda/code/nteract/applications/play/node_modules/graceful-fs/graceful-fs.js:78:16
    at FSReqWrap.readFileAfterClose [as oncomplete] (fs.js:511:3)

 ERROR  Failed to compile with 2 errors                                  

Which I guess is due to Monaco being in TypeScript? Unsure. I do not know what to make of the nextjs example in https://github.com/superRaytin/react-monaco-editor/tree/master/examples/nextjs. The component wrapper sets things in the window object and hardcodes a baseUrl, which make me queasy.

rgbkrk commented 6 years ago

It looks like the wrapper component shipping non-transpiled JS in that package. We only build / transpile our own packages so to make this work we'd have to either make our webpack config / babel loader transpile that package or fork the component (or write our own).

It looks like there's another option too. In the example next config they provide, they explicitly override fs to empty to make their package build. You'd have to add that to the next config for play as well.

yuvipanda commented 6 years ago

Last week has seen movement in the monaco packaging space, apparently! https://github.com/superRaytin/react-monaco-editor/issues/108 has more info and links, and is possibly the 'right' way to go long term?

On Tue, Apr 3, 2018 at 6:20 AM, Kyle Kelley notifications@github.com wrote:

It looks like the wrapper component shipping non-transpiled JS in that package. We only build / transpile our own packages so to make this work we'd have to either make our webpack config / babel loader transpile that package or fork the component (or write our own).

It looks like there's another option too. In the example they provide, they explicitly override fs to empty to make their package build. You'd have to add that to the next config for play as well.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/nteract/nteract/issues/2714#issuecomment-378247402, or mute the thread https://github.com/notifications/unsubscribe-auth/AAB23iFTd6m8mKQvKUnjCmcDdi959xvQks5tk3cqgaJpZM4TA4hX .

-- Yuvi Panda T http://yuvi.in/blog

rgbkrk commented 6 years ago

Oh nice, https://github.com/Microsoft/monaco-editor/pull/767 was a good read. Based on that, I'd say that we create our own wrapper component around Monaco, similar to what we have for codemirror.

stale[bot] commented 5 years ago

This issue hasn't had any activity on it in the last 90 days. Unfortunately we don't get around to dealing with every issue that is opened. Instead of leaving issues open we're seeking to be transparent by closing issues that aren't being prioritized. If no other activity happens on this issue in one week, it will be closed. It's more than likely that just by me posting about this, the maintainers will take a closer look at these long forgotten issues to help evaluate what to do next. If you would like to see this issue get prioritized over others, there are multiple avenues 🗓:

Thank you!

captainsafia commented 5 years ago

I'm gonna go ahead and transfer this to the new nteract/play repo.