14652 frontpage style #35
Conversation
The '+'/'-' usage was confusing so I split the lines. In addition, I amended println! to conform to the 4 spaces for indentation as stated in the style guide.
|
For the comments, maybe the point was to keep it short? I think assuming people know about the primitive arithmetic operations shouldn't be much unclear. |
|
To make it easy to see the changes, here's a screenshot: I think the playground button is too large (and too blue) and it looks a bit odd there. It can easily get in the way of user's code; the run button is small enough that I don't think this is a big issue for it. Further, I'm not sure expanding the comments in this way adds much information. I think we can assume our users will understand the previous comments. If anything, we could simply add ", respectively" to make it clear, so that the commentary becomes: As for resolving rust-lang/rust#14652, perhaps we could add a small "open external" icon next to the result that, when clicked, opens up the playpen with the code already in place. Like this:
|
|
@SergioBenitez Awesome. Do we need the respectively through? cc @brson |
|
@adrientetar @SergioBenitez Thank you for the feedback. I think I was trying to change the wording of the comments, but upon taking a second look at it, it doesn't seem as off as I thought. I don't think I've seen the usage of I think the "open external" icon makes a lot of sense - is the playground also hosted here? It seems you can send json requests to it but I'm not sure how to load the playground with an existing code sample. |
|
@jasonkliu This rustdoc PR uses the playpen API to open code on play.rust-lang.org: rust-lang/rust#14700 I agree with @SergioBenitez's suggestion about how to link to the playpen from the main example unobtrusively. |
|
@jasonkliu You can just append it as a query string, like this: http://play.rust-lang.org/?code=fn+main()+%7B%20println!(%22foo%22)%20%7D&run=1 Thus, grab the code from the editor, URL encode it, and append it to |
|
I've implemented the output button by taking the current input in the editor and putting it into the playground, running automatically. I've also fixed the comment text.
|
| function handleSuccess(message) { | ||
| resultDiv.style.backgroundColor = successColor; | ||
| resultDiv.innerHTML = escapeHTML(message); | ||
| var program = encodeURIComponent(editor.getValue()); | ||
| var output = "<a href=\"http://play.rust-lang.org/?code=" + program + |
There was a problem hiding this comment.
Mixing in HTML with JS isn't good style. Instead, place the HTML with the rest of the markup and simply change the HREF when the result is changed. Note: What do you do when there is an error, or when the result pane is open but the code in the editor has changed and hasn't been rerun?
|
@SergioBenitez: Thank you very much for the comments. I moved the html to the index file and used JS to replace the href of the link. I'm attempting to check whether the editor contents are the same as the generated HTML link as the link is clicked, but am running into issues. Using the // When clicking on the playground link, check if editor code is modified
function f1() {
var program1 = encodeURIComponent(editor.getValue());
var program2 = "http://play.rust-lang.org/?code=" + program1 + "&run=1"
if (program2 !== playLink.href) {
//playLink.href = program2
//window.location = program2
console.log(program2);
}
} |
|
@jasonkliu Sorry I've been away for a few days. What's left to do here? |
|
@brson Most of the functionality is done, with one small exception. The current workflows are:
In short, the method of processing involves taking the editor text and encoding it into a html link embedded in an icon. However, it is only updated when the "Run" button is clicked, so in case 4 above, the output button won't have an updated link when the user clicks on it. I tried to implement a function to check if the link href is not the same as the editor text (see above comment), but it doesn't work consistently since it is calling a function while the browser is navigating to a link.
|
|
@jasonkliu Thanks for the debrief. I've merged on the notion that this is a bug we can live with for a little while, but I would like to get it fixed. Can you try your first solution, and if that doesn't work, the second? |
|
@jasonkliu After merging the new code, when I click 'Run', the icon is just a unicode placeholder, not the desired icon. Can you investigate? |
|
@brson Regarding the icon issue, it happens to be fontello/fontello#156. Interestingly, the bug only appears in Firefox. I've switched the svg font to a woff font and tested with these browsers, which seem to work: 3135402 |
|
@jasonkliu Thanks for looking into it so quickly. You'll need to create a new PR. |
The issues addressed are rust-lang/rust#14651 and rust-lang/rust#14652.
Relative to 14651 (coding style of the frontpage), the println! line spacing was off. In addition, I updated the comments to make them more clear, since the current version doesn't seem intuitive to a new reader.
Relative to 14652, I added a button link to the playground. This repository seems to be the source for the main website; is the source for the playground also available on Github? Ideally, linking to the playground would copy the current code in the editor and paste it into the playground, but I'm not exactly sure how to do this without looking at the playground source. I would also like to implement different programs (such as Hello World) with a selector on the main page so additional code examples can be seen.