feat: adds cursor interactive examples #597

brianlmacdonald · 2018-02-20T19:13:06Z

Addresses #555

Adds cursor examples and a fun light blue div to mouse over, and in doing so, change your cursor's style.

welcome · 2018-02-20T19:13:09Z

💖 Thanks for opening this pull request! 💖
Here is a list of things that will help get it across the finish line: - If this is a new or updated CSS interactive example, please ensure that you followed the CSS styleguide - If this is a new or updated JavaScript interactive example, please ensure that you followed the JavaScript styleguide - If your changes affects any of the steps in our contribution docs, please also make the relevant changes there.

wbamberg

Thanks for your contribution @brianlmacdonald !

I had a few comments. Also, I don't think this example belongs on its own in a cursor directory, I think it should live in basic-user-interface, as that's the spec it's defined in.

wbamberg · 2018-02-21T00:49:48Z

live-examples/css-examples/cursor/cursor.css

+.example-choice-list {
+  height: 500px;
+  width: 250px;
+}


I don't think we should be adjusting the editor styles in this CSS (but actually, you don't need this anyway).

wbamberg · 2018-02-21T00:50:41Z

live-examples/css-examples/cursor/cursor.css

+
+.cursor {
+  display: flex;
+  background-color:#1766aa;


Should have a space here between the : and the value.

wbamberg · 2018-02-21T00:51:03Z

live-examples/css-examples/cursor/cursor.css

+  width: 250px;
+}
+
+.cursor {


I'm not sure why we don't simply apply this CSS to #example-element.

wbamberg · 2018-02-21T00:51:47Z

live-examples/css-examples/cursor/cursor.css

+  background-color:#1766aa;
+  color: white;
+  height: 100px;
+  width: 200px;


I'd suggest making this bigger, like 80% / 80%.

wbamberg · 2018-02-21T00:52:43Z

live-examples/css-examples/cursor/cursor.css

+  justify-content: center;
+  align-items: center;
+  font-size: 14pt;
+}


We use 4 spaces to indent (sorry not to have a style guide linked from the contribution docs).

wbamberg · 2018-02-21T00:57:10Z

live-examples/css-examples/cursor/cursor.html

+    <button type="button" class="copy hidden" aria-hidden="true">
+      <span class="visually-hidden">Copy to Clipboard</span>
+    </button>
+  </div>


We should restrict the number of choices to 6, to avoid scrolling. Which 6 values you pick is up to you, but try to choose a representative/commonly used set. I know it's hard in a case like this when there are so many values.

wbamberg · 2018-02-21T00:57:47Z

live-examples/css-examples/cursor/cursor.html

+    <div class='cursor 'id="example-element">Mouse over to see cursor style</div>
+  </section>
+
+</div>


File should end with a newline.

@wbamberg done and done. Thanks!

wbamberg · 2018-02-21T00:58:24Z

live-examples/css-examples/cursor/cursor.html

+<div id="output" class="output hidden large">
+
+  <section id="default-example" class="default-example container">
+    <div class='cursor 'id="example-element">Mouse over to see cursor style</div>


If you make the change I suggested in the CSS file you don't need this 'cursor' class.

wbamberg · 2018-02-21T00:59:32Z

live-examples/css-examples/cursor/cursor.html

+<div id="output" class="output hidden large">
+
+  <section id="default-example" class="default-example container">
+    <div class='cursor 'id="example-element">Mouse over to see cursor style</div>


In similar examples we end the sentence with a period and refer to the element explicitly, like: "Move over this element to see the cursor style."

brianlmacdonald · 2018-02-21T14:30:18Z

@wbamberg would a eslint-config for mdn's style be something useful/helpful? If so I could try my hand at making one for you all. It could run as a precommit script and would probably make your review process a bit more streamlined.

wbamberg · 2018-02-21T17:00:08Z

would a eslint-config for mdn's style be something useful/helpful?

Yes, very! @schalkneethling , is there a style guide that @brianlmacdonald could use for this?

wbamberg

Looks great @brianlmacdonald , just a couple of other things. There's an extra level of <div> nesting in the HTML, and the "cursor" directory and its contents should be removed now the example is in "basic-user-interface".

Thanks!

wbamberg · 2018-02-21T17:06:52Z

live-examples/css-examples/basic-user-interface/cursor.html

@@ -0,0 +1,56 @@
+<section id="example-choice-list" class="example-choice-list" data-property="cursor">
+  <div>


There's an extra <div> here that should be removed.

wbamberg · 2018-02-21T17:07:04Z

live-examples/css-examples/basic-user-interface/cursor.html

+    </button>
+  </div>
+
+</div>


Here's the other end of that <div>.

wbamberg · 2018-02-21T17:07:29Z

live-examples/css-examples/cursor/cursor.css

@@ -0,0 +1,11 @@
+#example-element {


You still need to remove these files under cursor.

wbamberg

This looks great.

It usually takes a couple of hours for the new page to be deployed, and I'll update MDN soon after that.

Thanks for your contribution @brianlmacdonald !

welcome · 2018-02-21T17:41:48Z

Congrats on merging your first pull request! 🎉🎉🎉

brianlmacdonald · 2018-02-21T17:44:04Z

Wahoo! @wbamberg Let me know if you'd like me to open the Eslint-config as an issue.

schalkneethling · 2018-02-21T17:44:49Z

Yes, very! @schalkneethling , is there a style guide that @brianlmacdonald could use for this?

@wbamberg @brianlmacdonald So, there are a couple of things.

The eslintrc for this project - https://github.com/mdn/interactive-examples/blob/master/.eslintrc.js
The eslintrc for Kuma(MDN Web Docs) - https://github.com/mozilla/kuma/blob/master/.eslintrc.js
And then there is a code styleguide in the works here that will also cover all these things.

With all of that said, there is a couple of things I am doing at the moment over and above the above ;)

Merging the eslintrc from Kuma and the interactive examples
Ensure that both projects have a pre-commit hook that lints the relevant JS
As soon as this PR[1] is merged, I am going to decouple the build tool from this project. One of the first items on the list to improve the builder to lint the example JS and CSS when pages are built so things are flagged at dev/build time.

wbamberg · 2018-02-21T21:42:37Z

I just updated the page -> https://developer.mozilla.org/en-US/docs/Web/CSS/cursor

feat: adds cursor interactive examples

98008b8

wbamberg suggested changes Feb 21, 2018

View reviewed changes

brianlmacdonald added 2 commits February 20, 2018 20:36

fix: refactors style

3489025

fix: moves cursor to correct directory

72d12f9

wbamberg suggested changes Feb 21, 2018

View reviewed changes

removes div and deletes cursor folder files

2e0b132

wbamberg approved these changes Feb 21, 2018

View reviewed changes

wbamberg merged commit 59cba29 into mdn:master Feb 21, 2018

wbamberg mentioned this pull request Feb 22, 2018

Logical properties #598

Merged

		@@ -0,0 +1,56 @@
		<section id="example-choice-list" class="example-choice-list" data-property="cursor">
		<div>

feat: adds cursor interactive examples #597

feat: adds cursor interactive examples #597

Uh oh!

Conversation

brianlmacdonald commented Feb 20, 2018

Uh oh!

welcome bot commented Feb 20, 2018

Uh oh!

wbamberg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brianlmacdonald commented Feb 21, 2018

Uh oh!

wbamberg commented Feb 21, 2018

Uh oh!

wbamberg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wbamberg left a comment

Choose a reason for hiding this comment

Uh oh!

welcome bot commented Feb 21, 2018

Uh oh!

brianlmacdonald commented Feb 21, 2018

Uh oh!

schalkneethling commented Feb 21, 2018

Uh oh!

wbamberg commented Feb 21, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants