483664 - Link "featured", "recommended", "experimental" labels to faq

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Public Pages, defect)

Comment 1

[Jeff Balogh (:jbalogh)]

noob.

Comment 2

[Nick Nguyen [:osunick]]

Example:
http://skitch.com/osunick/bejyi/search-add-ons-firefox-add-ons

These labels should point to the appropriate anchors in the AMO FAQ when clicked:
https://preview.addons.mozilla.org/en-US/firefox/pages/faq

Comment 3

[Jeff Balogh (:jbalogh)]

Attachment: linking featured and recommended labels to the faq

Comment 4

[Les Orchard [:lorchard]]

Hmm... This looks okay, but having trouble actually applying the patch.  Rather than a diff, it looks like an email of some sort

Comment 5

[Jeff Balogh (:jbalogh)]

The patch works for me from the root of the trunk checkout.  Patch ignores the mbox headers:

(khan) $ ls
bandwagon  bin  README  site
(khan) $ patch -p0<site/app/483664.patch
patching file site/app/views/addons/browse_thumbs.thtml
patching file site/app/views/addons/category_landing.thtml
patching file site/app/views/addons/display.thtml
patching file site/app/views/addons/searchengines.thtml
patching file site/app/views/addons/themes_landing.thtml
patching file site/app/views/addons/versions.thtml
patching file site/app/views/elements/addon_listitem.thtml
patching file site/app/views/helpers/addons_html.php
patching file site/app/webroot/css/color.css

Comment 6

[Les Orchard [:lorchard]]

Hurrr... okay, for some reason I kept trying patch -p1

Patch looks good, applied & works fine.  One thing though: I see that the "Experimental" header links to the faq, but not to the specific anchor.  That is, I see this:

http://dev.addons.mozilla.org/en-US/firefox/pages/faq

But not this:

http://dev.addons.mozilla.org/en-US/firefox/pages/faq#experimental-addons

Comment 7

[Jeff Balogh (:jbalogh)]

Attachment: linking featured and recommended labels to the faq

Linking to #experimental-addons and #recommended-addons (#recommended was added to the faq page).

Comment 8

[Les Orchard [:lorchard]]

Not seeing any changes between these two patches.

Comment 9

[Jeff Balogh (:jbalogh)]

Attachment: linking featured and recommended labels to the faq

With all the changes this time!

There's only two id="recommended-addons" because those are the only two locales that even have that section.

Comment 10

[Les Orchard [:lorchard]]

Comment on attachment 370228 [details] [diff] [review]
linking featured and recommended labels to the faq

Looks good to me!

Comment 11

[Jeff Balogh (:jbalogh)]

r24005

Comment 12

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using:

https://preview.addons.mozilla.org/en-US/firefox/pages/faq#recommended-addons
https://preview.addons.mozilla.org/en-US/firefox/pages/faq#experimental-addons

I read the FAQ, and understand that "Featured" are just "Recommended" add-ons; I don't think we have a "Featured" graphic anywhere (nor do I see a named anchor for "Featured" in the patch).

Please reopen/correct me if I'm wrong.

Comment 13

[Jeff Balogh (:jbalogh)]

Correct, no Featured.  I discussed this in real life with Nick, sorry to keep the internets out of the loop.

Comment 14

[Les Orchard [:lorchard]]

I did just notice one thing in this latest patch - line 506 of site/app/views/helpers/addons_html.php reads like this:

  } elseif ($addon['Addon']['recommended']) {

But, this throws a warning if DEBUG is on.  You should probably do something like:

  } elseif (!empty($addon['Addon']['recommended'])) {

That will do the same thing, but without a warning.

Comment 15

[Jeff Balogh (:jbalogh)]

Thanks, I found this yesterday too.  Fixed in r24034.