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

Make for-loop example of "Loops and iteration" guide an interactive one #14128

Closed
wants to merge 2 commits into from

Conversation

alattalatta
Copy link
Member

@alattalatta alattalatta commented Mar 21, 2022

Summary

Uses {{EmbedLiveSample}} for the for loop example.

Motivation

The example is quite complex with HTML markups and such. The code is long enough that the description of the example was too far. (The description - HTML markups - actual JS code)

This PR separates HTML and JS, and adds {{EmbedLiveSample}} so readers can try it. Also fixed three small things:

  • A {{jsxref("statements/for","for")}} loop repeats until a specified condition evaluates to false.

    • It doesn't have to be a false literal. Removed <code> around it.
  • If the value of conditionExpression is true, the loop statements execute. If the value of condition is false, the for loop terminates.

    • There is no condition, only conditionExpression.
  • Removed redundant values for HTML boolean attributes, such as multiple="multiple".

Supporting details

Related issues

Metadata

  • Adds a new document
  • Rewrites (or significantly expands) a document
  • Fixes a typo, bug, or other error

@alattalatta alattalatta requested a review from a team as a code owner March 21, 2022 12:26
@alattalatta alattalatta requested review from sideshowbarker and removed request for a team March 21, 2022 12:26
@github-actions github-actions bot added the Content:JS JavaScript docs label Mar 21, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Mar 21, 2022

Preview URLs

Flaws

None! 🎉

External URLs

URL: /en-US/docs/Web/JavaScript/Guide/Loops_and_iteration
Title: Loops and iteration
on GitHub

No new external URLs

(this comment was updated 2022-03-22 09:30:24.975007)

Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

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

I like the way this separates out the HTML and JS, but I'd like to know what @Elchi3 thinks of the live sample. As a rule we almost never use live samples in the JS docs, so it looks odd to see one here, especially as we don't have any for any of the other items.

@hamishwillee
Copy link
Collaborator

@wbamberg FWIW I'm also interested in the response here. I have wondered why we didn't use live samples in javascript docs. If anything, that feels like a bug to me :-)

If we do end up allowing examples, I personally prefer in-place logs of results to popups, and generally inline layouts. So something like:
image

@sideshowbarker sideshowbarker removed their request for review March 22, 2022 01:59
@Elchi3
Copy link
Member

Elchi3 commented Mar 22, 2022

I like the way this separates out the HTML and JS, but I'd like to know what @Elchi3 thinks of the live sample. As a rule we almost never use live samples in the JS docs, so it looks odd to see one here, especially as we don't have any for any of the other items.

When I rewrote all of the MDN JS docs for ECMAScript 6 in 2014/2015, there wasn't really a live sample system. And when we got it, I wasn't really convinced as it seemed to me like a hack with the sections and the iframe... Fasting forward to 2022, I don't think it is that bad anymore but I would still like it if MDN had one reliable example system. (And not 3 or 4 [live samples, interactive-examples, GitHub iframe examples, ...]. I think I prefer the interactive-examples system the most out of all, though it should be inline and not in an external repo but that's another rabbit hole).

If we do end up allowing examples, I personally prefer in-place logs of results to popups, and generally inline layouts. So something like:

Yes, strong +1. Over the years, I tried to replace alert with console.log calls.

@alattalatta
Copy link
Member Author

Well if so, I'll adjust the example to print it inline rather than alert. Other examples also deserve update (especially the labeled statements...) but they deserve separate PRs. 😄

@wbamberg
Copy link
Collaborator

I'm pro using live samples, I think. One slightly non-obvious reason is that they are more likely to work, because people will run them and report errors - it's a very unreliable form of distributed manual testing.

But I'm also very pro consistency. So I don't think we should merge this PR on its own - I think it would be really odd to have just one live sample in this page.

I would be happy to have a project like "add live samples to the JS Guide", and have a PR like this (or one that updated the whole page) as part of that project, with someone committed to seeing the project through.

I would still like it if MDN had one reliable example system

We have a project proposal about this, kind of! openwebdocs/project#59.

The thing about this though is that different example systems we have are good at different things, and inventing one that's the best at everything is quite hard. Not to mention updating all the thousands of pages to use it. So that seems like a long way off.

@alattalatta
Copy link
Member Author

Would it be better to close this and open another PR with the HTML/JS separation, but without the live sample?
Or, should I strip the example down so that it can be more in line with other ones, i.e. no HTML code, no DOM API access, only raw for loop?

@wbamberg
Copy link
Collaborator

Would it be better to close this and open another PR with the HTML/JS separation, but without the live sample? Or, should I strip the example down so that it can be more in line with other ones, i.e. no HTML code, no DOM API access, only raw for loop?

I like the separation of HTML and JS, and would happily merge a PR that did that. I think it's worth having the HTML. Thanks for your understanding @alattalatta !

For the bigger issue, I started https://github.com/mdn/content/discussions/14191 but noone commented...yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:JS JavaScript docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants