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

Added an example for String.length #1080

Merged
merged 12 commits into from Oct 4, 2018
Merged

Added an example for String.length #1080

merged 12 commits into from Oct 4, 2018

Conversation

irenesmith
Copy link
Contributor

No description provided.

@wbamberg wbamberg self-requested a review August 4, 2018 04:37
Copy link
Contributor

@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.

Thanks @irenesmith .

Just some formatting nits. But I also thought, since this is a short example, it might be worth including something a bit more advanced, like:

// Some characters are represented using 2 UTF-16 code units
var grinningFace = '😀';
console.log(grinningFace.length);
// expected output: "2"

<code id="static-js">const str='Life, the universe and everything. Answer:';
console.log(str + ' ' + str.length); // 42
</code>
</pre>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please end files with a newline.

@@ -0,0 +1,5 @@
<pre>
<code id="static-js">const str='Life, the universe and everything. Answer:';
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Also please use spaces for readability: var str = '...

@@ -0,0 +1,5 @@
<pre>
<code id="static-js">const str='Life, the universe and everything. Answer:';
console.log(str + ' ' + str.length); // 42
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use the standard expected output: format.

Copy link
Contributor

@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.

Thanks @irenesmith ! Still a couple of nits to fix here. Also, what did you think of the suggestion to also use an emoji character, to show that length is actually giving us UTF-16 units, not necessarily characters?

@@ -0,0 +1,7 @@
<pre>
<code id="static-js">const str = 'Life, the universe and everything. Answer:';
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be var not const.

live-examples/js-examples/string/string-length.html Outdated Show resolved Hide resolved
@irenesmith
Copy link
Contributor Author

You should be picky. Don't know why I said the wrong thing. Resolved both of these comments.

Copy link
Contributor

@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.

One more nit. Also, I had a suggestion about also using an emoji character, what did you think?

<code id="static-js">var str = 'Life, the universe and everything. Answer:';

console.log(str + ' ' + str.length);
// expected output: 42
Copy link
Contributor

Choose a reason for hiding this comment

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

expected output should be "Life, the universe and everything. Answer: 42"

Copy link
Contributor

@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.

Looks great, thanks @irenesmith . I thought more about the emoji example and decided to omit it, let's keep things simple. But I did make the editor shorter, since this is such a small example.

@wbamberg wbamberg merged commit c49d736 into mdn:master Oct 4, 2018
wbamberg pushed a commit to wbamberg/interactive-examples that referenced this pull request Oct 16, 2018
* upstream/master: (36 commits)
  Adds h1-h6 example (mdn#1180)
  chore(deps): update dependency eslint to v5.7.0 (mdn#1181)
  chore(deps): update dependency ajv to v6.5.4 (mdn#1178)
  chore(deps): pin dependency mdn-bob to 1.1.16 (mdn#1177)
  Removing some folders that was leftover (mdn#1179)
  Issue#1005 web api examples editor (mdn#1142) (mdn#1174)
  chore(deps): update dependency husky to v1.1.1 (mdn#1173)
  chore(deps): update dependency puppeteer to v1.9.0 (mdn#1172)
  Adding an example for the String.toString() method (mdn#1076)
  Adding method for String.valueOf() method (mdn#1078)
  Added an example for String.length (mdn#1080)
  Clarifying Promise.resolve() Example (mdn#1125)
  Remove background from datalist example (mdn#1099)
  Remove background from option example (mdn#1102)
  Removed background image from optgroup (mdn#1101)
  Update <img> to use CC0 image (mdn#1087)
  use `+=` instead of `a = a + x` (mdn#1171)
  chore(deps): update dependency husky to v1.1.0 (mdn#1168)
  chore(deps): update dependency eslint to v5.6.1 (mdn#1167)
  chore(deps): update dependency stylelint to v9.6.0 (mdn#1165)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants