Implement new Top Tags page

VERIFIED FIXED in 5.3.1

Status

addons.mozilla.org Graveyard
Public Pages
P3
normal
VERIFIED FIXED
9 years ago
2 years ago

People

(Reporter: clouserw, Assigned: wenzel)

Tracking

unspecified
5.3.1

Details

(URL)

Attachments

(3 attachments)

(Reporter)

Description

9 years ago
The new top tags page was designed in bug 513085 and the winning mockup is attachment 405200 [details].
Assignee: nobody → fwenzel
(Assignee)

Comment 1

9 years ago
I have a patch for this almost ready.
Status: NEW → ASSIGNED
(Assignee)

Comment 2

9 years ago
Created attachment 410642 [details] [diff] [review]
Patch, rev. 1

Here you go. Lots of jQuery and CSS <3.
Attachment #410642 - Flags: review?(clouserw)
(Assignee)

Comment 3

9 years ago
Created attachment 410643 [details]
Screenshot

... and here's a screenshot, so you (and QA) know what it's supposed to look like.
(Reporter)

Updated

9 years ago
Attachment #410642 - Flags: review?(clouserw) → review-
(Reporter)

Comment 4

9 years ago
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.
(Reporter)

Updated

9 years ago
Target Milestone: 5.3 → 5.4
(Assignee)

Comment 5

9 years ago
(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.
(Assignee)

Comment 6

9 years ago
Created attachment 410748 [details] [diff] [review]
Patch, rev. 2

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)
(Reporter)

Comment 7

9 years ago
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+
(Assignee)

Comment 8

9 years ago
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.
(Assignee)

Comment 9

9 years ago
Committed to r55702, r55703, and r55704. Some cleanup of the horribly misformatted existing code in r55705.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 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.