422995 - Add microformats to review pages

Status: RESOLVED DUPLICATE of bug 563344

(addons.mozilla.org Graveyard :: Public Pages, enhancement, P5)

Description

[Ryan Doherty (:rdoherty)]

Since yahoo now supports microformats when spidering and they are popular with the kids these days, I think we should add them to amo.

Right now the most appropriate places I can think of are reviews, user profiles and home links.

http://microformats.org/wiki/Main_Page

Comment 1

[Brian Krausz]

Taking this, will begin work on it next week.

Comment 2

[Brian Krausz]

Yea, so I lied about the "next week" thing.  But here I am!

Ryan: Can you clarify what you mean by "home links".

User profiles, while there isn't much find, can use hCard I suppose.  Reviews of course fit the hReview spec nicely.

Comment 3

[Brian Krausz]

Never got to this...reassigning to Ryan.

Comment 4

[Ryan Doherty (:rdoherty)]

Assigning to Smita. Would like to get this out the door in 4.0.3. 

Smita, let me know if you have any questions on implementation.

Comment 5

[smita]

Attachment: Patch for Bug 422995

Added microformats for user profiles and reviews.

For reviews added microformats at two places:
1. Place where all the reviews get displayed
2. Addon display page where only first few reviews get displayed.

Comment 6

[Ryan Doherty (:rdoherty)]

Comment on attachment 345236 [details] [diff] [review]
Patch for Bug 422995

There's a lot of things here that could have been done simpler. 

Overall I thought that this bug would only need a few microformat classnames added to the html and not so much code rewritten or removed.

I'm confused why so much code was removed from reviews/display.thtml? From line 90-120 in the patch file, lots of necessary code was removed. Clicking on 'report this review' now does nothing. It should be a dropdown menu instead of a link.

For users/info.thtml, the html tags shouldn't have been change from a definition list to generic spans and divs. Just the microformat classnames could have been added.

Example:
<dl class="vcard"> instead of <div class="vcard">

For addons/display.thtml, the translation title was changed from h5 to h4? Why? The translation body was also change from a <p> to <div> when a <p> is the correct semantic tag to use.

I'd recommend starting from scratch and only adding CSS classnames to html elements. Only if there aren't any elements to add classnames to you can add a div or span.

Comment 7

[smita]

I didn't remove any code from any files.By mistake I replaced previous version file with the new one,hence those lines are missing from reviews/display.thtml. I will take of such things in the future.Sorry about that.

As per your comments,I will just add the microformat class names to the existing html elements.

Comment 8

[smita]

Attachment: Updated Patch after review comments

Created a new patch. Updated following files
users/display.html
--added microformat classes to existing html elements.

reviews/display.html and addons/display.html
--had to add new div and span tags as there were no html tags where in I could put microformat classes,except for one or two html elements.

Also 'report this review' works fine.

Comment 9

[smita]

Sorry got the wrong file names,its users/info.thtml,reviews/display.thtml and addons/display.thtml

Comment 10

[Ryan Doherty (:rdoherty)]

Comment on attachment 346198 [details] [diff] [review]
Updated Patch after review comments

This is looking a lot better. Just a few small tweaks.

I don't think the classes item & vcard are necessary for the hreview description?

I don't think the developer replies can be considered hreviews. (views/reviews/display.thml:163)

And for the hreview rating class, I think a number is needed inside of it somewhere. Might need to use some CSS to hide it though.
http://microformats.org/wiki/hreview#Movie_Review

Comment 11

[smita]

Attachment: Patch for 422995 --updated

Updated the files after review comments.
The comment about hreview rating class is not very clear to me.
In the example given here:"http://microformats.org/wiki/hreview#Movie_Review"
they have used numbers to display the ratings,but we have used an image file.
Please let me know, if it should be done according to the example given.

Comment 12

[smita]

Attachment: Updated patch

Added 'ratings' as per the example given at http://microformats.org/wiki/hreview#Movie_Review

Comment 13

[Ryan Doherty (:rdoherty)]

Comment on attachment 348942 [details] [diff] [review]
Updated patch 

This is looking really good, just a few minor things to change.

The rating value should have a <span class="rating"> around it:
<span class = "ratings"><?=$review['Review']['rating']?> out of 5</span>

I also noticed on the addon detail page (example: https://addons.mozilla.org/en-US/firefox/addon/2207) there isn't anything marking up the review description.

Comment 14

[Fred Wenzel [:wenzel]]

(In reply to comment #13)
> The rating value should have a <span class="rating"> around it:
> <span class = "ratings"><?=$review['Review']['rating']?> out of 5</span>

Also, that needs to be localizable (chances are that string already exists, by the way).

Comment 15

[Ryan Doherty (:rdoherty)]

(In reply to comment #14)
> (In reply to comment #13)
> > The rating value should have a <span class="rating"> around it:
> > <span class = "ratings"><?=$review['Review']['rating']?> out of 5</span>
> 
> Also, that needs to be localizable (chances are that string already exists, by
> the way).

Thanks for catching that Wenzel!

Comment 16

[smita]

Sorry, but I am not clear about comment #14.Could you please help me understand.

Comment 17

[Ryan Doherty (:rdoherty)]

(In reply to comment #16)
> Sorry, but I am not clear about comment #14.Could you please help me
> understand.

All strings need to be localized using gettext. It's done via the _() calls like _('addon_review_admin_delete') which pass the id of the string to display.

Wenzel is correct, we have that string already: 'stars_rated_x_outof_5'. Shouldn't be too hard to sprintf() the rating into it.

Localizations are stored in locale/LOCALE/LC_MESSAGES/messages.po

Comment 18

[Ryan Doherty (:rdoherty)]

Comment on attachment 347729 [details] [diff] [review]
Patch for 422995 --updated

Pretty sure this is an older patch that needed to be r-'d

Comment 19

[smita]

Attachment: Updated Patch for bug 422995

Updated patch after incorporating changes suggested in comment# 13 and comment# 14.

Comment 20

[Ryan Doherty (:rdoherty)]

Comment on attachment 350436 [details] [diff] [review]
Updated Patch for bug 422995

This has the localizable string, but according to 
http://microformats.org/wiki/hreview the rating should be wrapped with a class "rating"

Example:
<span><span class="rating">5</span> out of 5 stars</span>

Comment 21

[Wil Clouser [:clouserw]]

smita: are you still working on this?

Comment 22

[Jeff Balogh (:jbalogh)]

Forward-duping because I love my bugs so much more.