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

Embed a playpen-like editor inside the book #73

Closed
4 tasks done
japaric opened this issue Jun 3, 2014 · 28 comments · Fixed by #85
Closed
4 tasks done

Embed a playpen-like editor inside the book #73

japaric opened this issue Jun 3, 2014 · 28 comments · Fixed by #85

Comments

@japaric
Copy link
Member

japaric commented Jun 3, 2014

I got this working [1][2] in the editor branch and have setup a clone site for testing.

At the moment, I have two bugs to fix:

  • The buttons are misplaced, they should be inside the editor area
  • You need to refresh the page to see the editor
  • Use the same colors when highlighting both static code and the code inside the editor.
  • Height of the editor should always fit the contained source code.

I'll like to have other people test it, and let me know if you have any suggestion or if you find any other bug. (or if you know how to fix any of the bugs I have mentioned :P - here's the css and the js if you want to take a look)

Once we are sure the editor works the way we want, I'll merge this into the original site.

[1] I've reused @SergioBenitez live code editor implementation for the rust-lang.org website. Kudos to him!
[2] Implementation detail: The "{hello.play}" string in the input markdown gets expanded into the live code editor containing the hello.rs source code

cc @kud1ing @vojtechkral @vks @zslayton


Original description

The playpen exposes a CORS-enabled JSON API that could be used to embed a playpen-like editor in each example, this would allow the user to run the example, and see the output inline; or experiment further by editing the source code of the example.

See rust-lang/prev.rust-lang.org#26 for a reference implementation. We should try to provide a similar interface to the one provided by rust-lang.org (plus a "Reset" button to show the original example code)

This was referenced Jun 3, 2014
@vks
Copy link
Contributor

vks commented Jun 6, 2014

Except for the two bugs you mentioned it seems to work fine.
One thing I noted when trying a few of the examples was that there are compiler warnings because of unused variables.
I think we should prefix them with _ to silence the warnings. (Especially since the concerned lines are getting highlighted automatically.)

The syntax highlighting/look is different from the static code listings. This probably should be fixed at some point.

@kud1ing
Copy link
Contributor

kud1ing commented Jun 6, 2014

Except for the two bugs you mentioned it seems to work fine.

I agree, good work. It would be great, if the height of the editor would be automatically adapted to the height of its content. Currently i see quite some visual gap between those 5 lines of Hello World and its output below.

@vojtechkral
Copy link
Contributor

Looks great.

The refresh is needed because the editor seems to be initialized simply in top-level code in editor.js

@japaric
Copy link
Member Author

japaric commented Jun 6, 2014

One thing I noted when trying a few of the examples was that there are compiler warnings because of unused variables.

I have opened #83 to track this.

The syntax highlighting/look is different from the static code listings. This probably should be fixed at some point.

There should be some theme common to gitbook and the ace editor, I'll switch to that theme, to make the syntax highlighting consistent.

It would be great, if the height of the editor would be automatically adapted to the height of its content.

It already adapts to the height of the source code, but I have set a minimum height for the editor, because the editor doesn't grow in height when you insert more code.

If I remove the minimum height, then, for example, the "hello" editor will have 5 lines of code, if you type more lines in the editor then you'll have to scroll to see other sections of the code, which can be a little annoying.

@SergioBenitez
Copy link

@japaric You can use the exact same resize code to resize the editor automatically when the user's code exceeds the height of the editor (so that the box is always big enough to fit the code). You can do this by adding the following line (probably at the end of the file):

editor.getSession().on('change', updateEditorHeight);

To fix the refresh issue, you'll need to figure out which function gets called when the page changes. Then, move the editor initialization code into one function (including the event handlers) and call that function from the page change function.

@vojtechkral
Copy link
Contributor

@japaric I fixed the refresh bug, would you like a PR?

[I don't do this very often, not sure how this works yet :-) ]

@japaric
Copy link
Member Author

japaric commented Jun 6, 2014

@SergioBenitez Thanks for the advice, I'll try that.

I fixed the refresh bug, would you like a PR?

\o/ Sure, send a PR to the editor branch.

[I don't do this very often, not sure how this works yet :-) ]

Do you mean sending a PR? or how you fixed the refresh issue?

@vojtechkral
Copy link
Contributor

I meant sending a PR while another PR from me is queued, I'm not sure what that will do. But I hope it's ok since it's on another branch...

@japaric
Copy link
Member Author

japaric commented Jun 6, 2014

@vojtechkral roughly speaking 1 PR = 1 branch, so you'll have to fetch my editor branch, then create another branch on top of the editor branch (named, for example, as fix-refresh branch), and submit a PR of your fix-refresh against my editor branch.

@vojtechkral
Copy link
Contributor

Ok, will do...

EDIT: done, hope it's ok

@japaric
Copy link
Member Author

japaric commented Jun 6, 2014

I updated the preview site.

@vks The colors of the editor and the static code now match
@kud1ing The editor is now resizable (thanks @SergioBenitez)
@vojtechkral fixed the refresh issue

The only thing left to fix are the buttons (they moved, but not where I wanted them to be :P)

@vojtechkral
Copy link
Contributor

There's one more hiccup: The editor theme is not changed along with gitbook's theme changes. Trouble is, gitbook doesn't provide any event on that, so the solution would be a bit hacky...

@japaric
Copy link
Member Author

japaric commented Jun 6, 2014

@vojtechkral Gitbook provides an exercise feature that uses an ACE editor (like we do here), but they don't provide a method to change the theme of the editor when the theme of the book changes. If/when they provide a method for that, we'll use that here.

@vojtechkral
Copy link
Contributor

Ok :-)

@japaric
Copy link
Member Author

japaric commented Jun 7, 2014

I fixed the position of the buttons and updated the preview site

If no one has another suggestion/objection, I'll merge this.

@vojtechkral
Copy link
Contributor

Some of the examples don't work for some obscure reason (eg. Formatted print, Tuples,..), it says:

playpen: getpwnam: No such file or directory
playpen: application terminated with error code 1

@japaric
Copy link
Member Author

japaric commented Jun 7, 2014

Some of the examples don't work for some obscure reason (eg. Formatted print, Tuples,..), it says:

playpen: getpwnam: No such file or directory
playpen: application terminated with error code 1

The playpen application seems to be malfunctioning at the moment. I think only examples that have been cached are working.

Better ping @thestinger

@kud1ing
Copy link
Contributor

kud1ing commented Jun 7, 2014

Looks very good, thank you guys.

@thestinger
Copy link

@japaric: It works fine for me. There are automated upgrades and perhaps one of them went poorly.

@thestinger
Copy link

Yes, it looks like a network connection failed and the Bash script updating it doesn't do error checking.

@japaric
Copy link
Member Author

japaric commented Jun 7, 2014

@thestinger good to know it's working again

Thank you all guys for the feedback, I have merged this in the master branch. Once I finish with #80, I'll update the site.

@chriskrycho
Copy link
Contributor

Sort of follow up to this: I found this issue while looking around for how to go about implementing the theme change (because reading at night, it's killing my eyes). Anybody know if they added that functionality? Or at least a pointer to where to look?

I'd be happy to make whatever changes are necessary on this end to support if they have.

@mdinger
Copy link
Contributor

mdinger commented Aug 4, 2015

Which part? How to change the playpen theme or how to expose it to the webpage as an option?

@chriskrycho
Copy link
Contributor

Either or both! My initial looking around at the GitBook implementation
didn't make it clear, though I'm also happy to spend more time digging
on the docs in the next couple days.

@mdinger
Copy link
Contributor

mdinger commented Aug 4, 2015

Well, changing the theme isn't actually that hard. See mdinger@767361b where I change the theme to "tomorrow night" (it still flashes white so I probably missed a css value somewhere though). Valid theme names are listed at ace. Both the rust playpen and the ace kitchen sink demo support changing themes so you can browse their themes for something you like if that helps.

To use that though, it should probably change to a dark theme when you switch gitbook to a dark theme so you'd have to signal the change somehow and then modify that and all the colors and warnings and such then.

@chriskrycho
Copy link
Contributor

Right, and it was hooking into the GitBook events that I was concerned
with; my initial question wasn't clear enough! Does GitBook expose
that event programmatically? Otherwise, can we maybe just add a little
on-page JS to watch another CSS class, and on that basis update the
style sheet reference? I'll take another look at how the Ace style
sheet is attached.

@mdinger
Copy link
Contributor

mdinger commented Aug 4, 2015

That I don't know. I think I've looked at doing it before but never was sure how to proceed.

@chriskrycho
Copy link
Contributor

I’ll take a further look over the course of the week and see if I can see a good way to hook into that event-wise and substitute the CSS. If nothing else, it looks like the style used is just set on the editor div itself like ace-tomorrow (good for them for using it sensibly!) so it should be pretty straightforward to update if there’s an event I can hook into. I may even—as a temporary workaround—be able to do it just by watching for changes to the main book container div. I’ll post a further update when I have more.

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.

8 participants