Closed Bug 523372 Opened 15 years ago Closed 15 years ago

Implement new Top Tags page

Categories

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

defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: clouserw, Assigned: wenzel)

References

()

Details

Attachments

(3 files)

The new top tags page was designed in bug 513085 and the winning mockup is attachment 405200 [details].
Assignee: nobody → fwenzel
I have a patch for this almost ready.
Status: NEW → ASSIGNED
Attached patch Patch, rev. 1Splinter Review
Here you go. Lots of jQuery and CSS <3.
Attachment #410642 - Flags: review?(clouserw)
Attached image Screenshot
... and here's a screenshot, so you (and QA) know what it's supposed to look like.
Attachment #410642 - Flags: review?(clouserw) → review-
Comment on attachment 410642 [details] [diff] [review]
Patch, rev. 1

It looks great, but clicking on the tags is slow, even on khan.  The data isn't going to change often, we should generate the popularity offline and query the cached data.
Target Milestone: 5.3 → 5.4
(In reply to comment #4)
> It looks great, but clicking on the tags is slow, even on khan.

I don't think it's the SQL query making a big difference here -- it's probably the lag from the AJAX call. I didn't want to stuff all tags' data into this page. I improved it a little locally by not regenerating the popup on each click, but preloading it, then just filling in the AJAX data on each click.

I also don't think we need additional database fields: The "useraddontags" (or so) table exists, and if we join that with the add-ons table, we have our result. I'll dump that into a custom SQL query, because as you've seen, it's hard to do in Cake.
Attached patch Patch, rev. 2Splinter Review
Here's the patch I was talking about. I am also showing a "loading" image now while waiting for AJAX. It feels very snappy now, because you immediately see something happening. The custom SQL query also reduces complexity.
Attachment #410748 - Flags: review?(clouserw)
Comment on attachment 410748 [details] [diff] [review]
Patch, rev. 2

I don't think it feels snappy, but alright.

When you commit, please add the loading image.

Also, you're adding a half dozen ^Ms to user_tag_addon.php that shouldn't be there.
Attachment #410748 - Flags: review?(clouserw) → review+
I'll need to hold off committing this until AMO 5.3 is deployed, right?

(In reply to comment #7)
> When you commit, please add the loading image.

Thanks, the URL was borken, I fixed it :(

> Also, you're adding a half dozen ^Ms to user_tag_addon.php that shouldn't be
> there.

I only *delete* 4 lines in my patch from that file, so I don't think I added any of those. Nonetheless, I shall clean up the mess.
Committed to r55702, r55703, and r55704. Some cleanup of the horribly misformatted existing code in r55705.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Keywords: push-needed
Resolution: --- → FIXED
Verified FIXED; I tested in:

* Firefox 3.0
* Firefox 3.5
* Firefox 3.6 beta 2
* IE 7
* IE 8
* Google Chrome
* Opera 10
Status: RESOLVED → VERIFIED
This bug was pushed live off-cycle tonight.
Keywords: push-needed
Target Milestone: 5.4 → 5.3.1
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: