Update error.xml #630

AurelioDeRosa · 2015-01-22T06:39:10Z

Fix issue #413.

dmethvin · 2015-01-22T13:27:03Z

entries/error.xml

@@ -52,6 +52,15 @@ $( "img" )
  })
  .attr( "src", "missing.png" );
 ]]></code>
+    <p>In the example, missing.png is an hypotetical missing image that is used to trigger the <code>error</code> event, not an image to replace those giving an error. To do that, you have to modify the body of the anonymous function passed to <code>.error()</code> as shown below:</p>


hypothetical

Typo corrected. Sorry about that.

Fixed typo

Added semicolon after the word "exists" as per request.

arthurvr · 2015-01-22T16:23:09Z

This PR looks good to me. Shall I merge?

arthurvr · 2015-02-06T18:37:26Z

entries/error.xml

+  })
+  .attr( "src", "missing.png" );
+]]></code>
+  <p><b>Note:</b> Be sure that the replacement image exists; otherwise the <code>error</code> event will be triggered indefinitely.</p>


the indentation for this line seems wrong.

Also, I don't think <p> tags are allowed in <example>s. This just results into this: http://gyazo.com/f7795ca0fd998b6701b68bbb322fed0d (notice your extra info inside the <p> isn't rendered).

@AurelioDeRosa Do you have a working local docs setup? Normally you should be able to repro that there. If you don't, ping me on IRC and I'll be happy to help you out.

ping @AurelioDeRosa

@arthurvr I'm going to take care of this issue right now. Sorry for the delay.

AurelioDeRosa · 2015-02-28T23:08:20Z

As @arthurvr noted, the current version of the XSL file doesn't allow to have a paragraph in an example. So, the only solution I see is to allow it. Who can fix this issue? Any alternatives? // cc @dmethvin @kswedberg @scottgonzalez

dmethvin · 2015-03-01T19:02:17Z

I think that notes were intended for small notes and not big clips with paragraphs and examples. If we allow big clips and block elements like code, it may affect layout in a drastic way so that's something to take into consideration.

scottgonzalez · 2015-03-02T17:38:26Z

Well you for sure can't just throw arbitrary markup into <example>. We can potentially add support for <longdesc> inside <example>, but the currently used markup will never be allowed.

AurelioDeRosa · 2015-03-02T17:44:44Z

What's your suggestion to rearrange the code to have a description?

scottgonzalez · 2015-03-02T17:49:09Z

That's hard to say because I can't tell what you're trying to do. Are you trying to create an inline runnable example that is accompanied by a code block explaining how to do something? This is very different from the way any other page is designed.

arthurvr · 2015-03-02T17:50:37Z

In this case I'd opt to move some stuff in the <desc> and other things can fit in a JavaScript comment.

We can potentially add support for inside , but the currently used markup will never be allowed.

This is kinda off-topic for this PR, and should be discussed in grunt-jquery-content isn't it?

scottgonzalez · 2015-03-02T18:23:42Z

In this case I'd opt to move some stuff in the and other things can fit in a JavaScript comment.

The <desc> is really a title. So I don't think that's an acceptable solution.

We can potentially add support for inside , but the currently used markup will never be allowed.

This is kinda off-topic for this PR, and should be discussed in grunt-jquery-content isn't it?

We certainly shouldn't open an issue for grunt-jquery-content until we know what we're actually discussing. I'd like to hear from @AurelioDeRosa since he sent the PR which contains two <code> blocks inside <example>, along with other HTML content.

AurelioDeRosa · 2015-03-02T18:48:44Z

@scottgonzalez The PR addresses #413. The idea is to have a description of what's going on in the example for the benefit of some users.

scottgonzalez · 2015-03-02T19:05:16Z

Sounds like what you really want is a new, useful example.

AurelioDeRosa · 2015-03-02T19:50:06Z

I'd not say that a new example will solve the problem. The problem was the user didn't get what the example really did. Perhaps @arthurvr's suggestion of having a few comments in the code can fix this.

AurelioDeRosa · 2015-03-06T16:18:44Z

@arthurvr @scottgonzalez Should I try to submit a PR which has JavaScript comment so that we can review it?

scottgonzalez · 2015-03-06T16:46:48Z

sure

arthurvr · 2015-03-27T18:42:27Z

I'd love to move forward on this PR. ping @AurelioDeRosa

Removed all the paragraphs inside the examples.

AurelioDeRosa · 2015-03-29T14:37:07Z

PR updated @arthurvr.

arthurvr · 2015-04-09T11:39:20Z

entries/error.xml

@@ -20,7 +20,7 @@
    </argument>
  </signature>
  <longdesc>
-    <p>This method is a shortcut for <code>.on( "error", handler )</code>.</p>
+    <p><b>As of jQuery 1.8</b>, the <code>.error()</code> method is deprecated. Use <code>.on( "error", handler )</code> to attach event handlers to the <code>error</code> event instead.</p>


This is a totally unrelated change. Mind extracting this to its own commit?

ping @AurelioDeRosa ;)

hey-hey @AurelioDeRosa still interested in picking this up?

ping @AurelioDeRosa

AurelioDeRosa · 2015-05-24T19:17:44Z

PR updated @arthurvr. Sorry for the late change.

arthurvr · 2015-05-24T19:21:37Z

@AurelioDeRosa I liked the change, it was just unrelated to the thing you're trying to fix, so it should have been in a separated commit. I didn't mean you should remove the change 😉

AurelioDeRosa · 2015-05-24T19:27:14Z

@arthurvr I was thinking about opening a different PR for it, that's why I removed it.

arthurvr · 2015-05-24T19:28:00Z

Makes sense. Thanks.

arthurvr · 2015-05-28T05:10:02Z

entries/error.xml

@@ -20,7 +20,6 @@
    </argument>
  </signature>
  <longdesc>
-    <p>This method is a shortcut for <code>.on( "error", handler )</code>.</p>


This paragraph shouldn't be removed (or at least not in this commit).

Oh... 🐑, you're right!

AurelioDeRosa · 2015-06-08T00:12:09Z

Can someone check the updated PR so that we can merge it?

arthurvr · 2015-06-08T04:47:41Z

entries/error.xml

  </longdesc>
  <example>
-    <desc>To hide the "broken image" icons for IE users, you can try:</desc>
+    <desc>To hide the "broken image" icons for users of older versions of IE, you can write:</desc>


of older versions of IE

Which versions does that affect?

I remember is IE <= 10 but I can't find any reference. Someone with a hint on this?

No idea, honestly. I do think though that just saying "older versions of IE" means nothing so we should figure this out. Maybe @dmethvin or @timmywil have links to relevant discussions about this?

ping @AurelioDeRosa @dmethvin @timmywil is there anyone who knows this? If not I'll quickly test this out.

Yeah, I mentioned elsewhere that I'm not sure when the "broken image" thing went away. Probably before IE9?

Should we specify IE9 for now and update in case of mistake?

AurelioDeRosa · 2015-09-20T23:31:11Z

This PR is standing from a few months. Is anyone happy to specify IE9 and merge it?

dmethvin · 2015-09-21T01:38:23Z

Looks like this has been settable from at least IE and Firefox for years:

http://stackoverflow.com/questions/1694937/firefox-show-broken-images
http://ccm.net/faq/13546-internet-explorer-show-image-download-placeholders

However, I couldn't find out when the default was changed. How about if we just delete the example? There is still a good one there.

AurelioDeRosa · 2015-09-23T21:42:30Z

@dmethvin Thank you for the feedback. I've updated the PR based on your comment.

arthurvr · 2015-10-07T15:04:40Z

lgtm 👍

Fixes jquerygh-143 Closes jquerygh-630

Update error.xml

b530614

Fix issue jquery#143.

dmethvin reviewed Jan 22, 2015
View reviewed changes

AurelioDeRosa added 2 commits January 22, 2015 13:35

Update error.xml

261c2ff

Fixed typo

Update error.xml

eefce55

Added semicolon after the word "exists" as per request.

arthurvr reviewed Feb 6, 2015
View reviewed changes

Update error.xml

5cbd2e5

Removed all the paragraphs inside the examples.

jquerybot added the CLA: Valid label Mar 29, 2015

arthurvr reviewed Apr 9, 2015
View reviewed changes

Error: Removed unrelated paragraph

995048f

arthurvr reviewed May 28, 2015
View reviewed changes

Update error.xml

087e4ef

arthurvr reviewed Jun 8, 2015
View reviewed changes

Deleted first example

7cf36b2

AurelioDeRosa self-assigned this Oct 7, 2015

AurelioDeRosa added a commit to AurelioDeRosa/api.jquery.com that referenced this pull request Oct 7, 2015

Error: Improved image example

92ea75f

Fixes jquerygh-143 Closes jquerygh-630

AurelioDeRosa closed this in ea61638 Oct 7, 2015

AurelioDeRosa deleted the patch-6 branch October 19, 2015 19:35

Update error.xml #630

Update error.xml #630

Conversation

AurelioDeRosa commented Jan 22, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arthurvr commented Jan 22, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AurelioDeRosa commented Feb 28, 2015

dmethvin commented Mar 1, 2015

scottgonzalez commented Mar 2, 2015

AurelioDeRosa commented Mar 2, 2015

scottgonzalez commented Mar 2, 2015

arthurvr commented Mar 2, 2015

scottgonzalez commented Mar 2, 2015

AurelioDeRosa commented Mar 2, 2015

scottgonzalez commented Mar 2, 2015

AurelioDeRosa commented Mar 2, 2015

AurelioDeRosa commented Mar 6, 2015

scottgonzalez commented Mar 6, 2015

arthurvr commented Mar 27, 2015

AurelioDeRosa commented Mar 29, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AurelioDeRosa commented May 24, 2015

arthurvr commented May 24, 2015

AurelioDeRosa commented May 24, 2015

arthurvr commented May 24, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AurelioDeRosa commented Jun 8, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AurelioDeRosa commented Sep 20, 2015

dmethvin commented Sep 21, 2015

AurelioDeRosa commented Sep 23, 2015

arthurvr commented Oct 7, 2015