456130 - Interactive Collections Template

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Collections, defect, P1)

Description

[Justin Scott [:fligtar]]

From the specs:
    *  The Mozilla Collections Template is a special skin and layout of the “web view” tailored to first-time add-on consumers with interactive add-on selection widgets.
    * This view has an introductory description at the top, followed by an accordion-like listing of tasks or categories of add-ons that can be expanded and collapsed.
    * Expanding a category shows the add-ons within that category, including their name, summary, preview image, and an add-to-shopping cart button.
    * The bottom of the page lists the names of all selected add-ons and has a bundle installation button.
    * [open issue – how are the categories stored/selected?]
    * [open issue – is this category/interactive format something we want regular users to be able to use? e.g., choose what template you want your collection to appear in]

Comment 1

[Fred Wenzel [:wenzel]]

IIRC I am waiting on template mocks. Assigning to boriss, please bounce the bug back to me once you have them.

Comment 2

[Jennifer Morrow [:Boriss] (UX)]

Thanks, I'm on it.

Comment 3

[Michael Morgan [:morgamic]]

Any progress on this?

Comment 4

[Fred Wenzel [:wenzel]]

Reassigning this bug: I am working on implementing the wireframes. I am still missing data, and it doesn't install anything yet, but the layout is almost done.

Comment 5

[Fred Wenzel [:wenzel]]

Attachment: Template, v1

Let's get an extra set of eyes on this. The template is done so far.

It has/is:
- all CSS, as far as I can tell
- semi-dynamicly populated (from the DB but not through the nonexistent collections data yet)
- installation should work

It is missing:
- collection data
- an extra popup to ask for confirmation and allow unchecking add-ons
- success message
- minor things to make it degrade gracefully without JS (like actually doing something on form submission)

Let me know how this looks so far. Thanks.

Comment 6

[Ryan Doherty (:rdoherty)]

Comment on attachment 343081 [details] [diff] [review]
Template, v1

Looks like the patch is missing collections.css

Comment 7

[Fred Wenzel [:wenzel]]

Attachment: Same patch, with CSS

Yeah, my bad. I just noticed that a minute ago as well. Try this.

Comment 8

[Ryan Doherty (:rdoherty)]

Comment on attachment 343100 [details] [diff] [review]
Same patch, with CSS

Looks pretty good, just a few notes.

<div class="amo-plug"> could be a <p> instead, semantically it's a paragraph or maybe a header.

<h1 class="collections-header"> could just be a <h1> and targeted with .collections #content-main h1 instead. Less classnames.

<h3 class="cat-name"> could be targeted with .cat-header h3. And it's closed with an </h1> instead of </h3>

<p class="cat-description"> could be targeted with .cat-header p

<h4 class="name"> is closed with a </h3> instead of a </h4>

Bonus points for extreme speed at getting this done so fast!

Comment 9

[Fred Wenzel [:wenzel]]

Thanks for helping me improve my CSS skills :) I'll make these changes.

Comment 10

[Jennifer Morrow [:Boriss] (UX)]

Sounds great Frederic - It seems you and I should be communication to actually getting the graphic design assets added.

Comment 11

[Fred Wenzel [:wenzel]]

All right, I made the suggested CSS changes locally.

Question: Is it consensus that we want a popup where you can again review your "shopping cart" and remove selections before finally triggering the installation process? I am asking because that will result in *two* confirmation clicks for installing (our popup, then the addon manager window). This seems a little overkill to me.

Sadly, you cannot disable single add-ons in the addon manager installation dialog, so if we find this to be vital, we will indeed need to go down that route. Otherwise, I suggest we just fire up the installation dialog directly when you click "install now".

Comment 12

[Michael Morgan [:morgamic]]

Fred, since this isn't quite ready yet we will have to push it on Oct. 23rd.  Could you be finished by Oct. 21?

Comment 13

[Fred Wenzel [:wenzel]]

(In reply to comment #12)
> Could you be finished by Oct. 21?

Probably, yes. It's not blocked on me, per se, there are still design/content/etc. questions popping up. Provided these get out of the way soon, this should be ready quickly.

Comment 14

[Ryan Doherty (:rdoherty)]

Comment on attachment 343100 [details] [diff] [review]
Same patch, with CSS

Also, could we just put the collections.css into screen.css and save the extra http request?

Comment 15

[Fred Wenzel [:wenzel]]

(In reply to comment #14)
> (From update of attachment 343100 [details] [diff] [review])
> Also, could we just put the collections.css into screen.css and save the extra
> http request?

Sure. Keep in mind though that the design is (apparently) not going to stay this way, and may even dramatically differ from the rest of the site.

Comment 16

[Fred Wenzel [:wenzel]]

Attachment: Template, with CSS fixes and confirmation dialog

Now it has a confirmation dialog. Note: Redirection to the success page when the installation is complete will only work if your dev host is whitelisted (i.e., in about:config->xpinstall.whitelist). That'll be the case on AMO, so no problems there.

Missing:
- data
- non-JS fallback (though it is set up in a non-JS friendly way)
- final design

Comment 17

[Fred Wenzel [:wenzel]]

TODO: browser sniffing.

Comment 18

[Ryan Doherty (:rdoherty)]

Comment on attachment 343563 [details] [diff] [review]
Template, with CSS fixes and confirmation dialog

Looks really good minus a few small details.

The modal dialog isn't centered. It's always moved to the right by about 100 extra pixels. Not really sure why though.

I don't think the:
<div class="clear"><span><!-- clear --></span></div>

is necessary. 

We could just do a 
.collections li.list-addon:after {
content:'.';
clear:both;
display:block;
line-height:0;
visibility:hidden;
width:0;
height:0;
}

Comment 19

[Fred Wenzel [:wenzel]]

Attachment: Overlay fixed size, centered; smaller previews

All right, thanks for pointing that out. The overlay is now centered, I reduced the preview size slightly (per drolnitzky's request) and made sure the previews stay left (there was a small flaw, compare that to your screenshot).

I added sniffing for anything but Fx3, which shows a warning. If you want to improve the wording there, be my guest :)

Comment 20

[Ryan Doherty (:rdoherty)]

Comment on attachment 343590 [details] [diff] [review]
Overlay fixed size, centered; smaller previews

Looks good!

Comment 21

[Fred Wenzel [:wenzel]]

(In reply to comment #20)
> (From update of attachment 343590 [details] [diff] [review])
> Looks good!

Checked in to r19187.

Comment 22

[Fred Wenzel [:wenzel]]

Attachment: SQL to add subcategory ID to addons_collections table

For this template, we need to be able to categorize the collection. This SQL will do that.

Comment 23

[Fred Wenzel [:wenzel]]

Attachment: Initial add-on list SQL

This is the initial list of add-ons, assigned to categories.

Comment 24

[Fred Wenzel [:wenzel]]

Attachment: Patch to feed categories from the database

This patch, instead of using dummy add-ons, pulls them from the DB.

I noticed a problem though: When you choose an add-on, its list item grows because of the "done choosing?" box. However, the individual category does not grow with it, but instead pushes the last add-on in the category under the next category. Any good idea how to fix this?

Comment 25

[Ryan Doherty (:rdoherty)]

(In reply to comment #24)
> I noticed a problem though: When you choose an add-on, its list item grows
> because of the "done choosing?" box. However, the individual category does not
> grow with it, but instead pushes the last add-on in the category under the next
> category. Any good idea how to fix this?

I was able to reproduce a similar bug where if you select the last add-on in a category, the bottom edge dissappears when the 'Done choosing add-ons?' box appears. It looks like the height of the add-ons list is being set by pixels by the accordion plugin instead of auto. It doesn't happen every time, it only seems like it happens after you open and close a few categories.

The height in pixels is necessary to do the animation, but if there's a callback for when the animation is done you could set the height back to auto.

Comment 26

[Ryan Doherty (:rdoherty)]

Comment on attachment 343643 [details] [diff] [review]
Patch to feed categories from the database

Looks good!

Comment 27

[Fred Wenzel [:wenzel]]

(In reply to comment #26)
> (From update of attachment 343643 [details] [diff] [review])
> Looks good!

Checked into r19203, thanks.

I'll update the add-on list SQL here shortly.

Also, the "accordion height" problem seems fixed in the new version of jquery UI that fligtar just updated to. Thanks, Fligtar!

Comment 28

[Fred Wenzel [:wenzel]]

Attachment: Revised add-on list

Comment 29

[Justin Scott [:fligtar]]

Fred, this is looking great. Could you have the SQL run on preview to add the add-ons?

This is a nit-picky thing, but it'd be cool if after someone clicks on a category if you could .blur() so it's not selected anymore.

Comment 30

[Fred Wenzel [:wenzel]]

(In reply to comment #29)
> This is a nit-picky thing, but it'd be cool if after someone clicks on a
> category if you could .blur() so it's not selected anymore.

I fought with jquery-ui for an hour or so until I found out how the undocumented API works :-/ but now, I blur() when you click a header, and also scroll to the appropriate place when it's done doing its magic: Before, it just stopped somewhere in the page, but not at the category you wanted to go to.

r19223.

I also filed bug 460849 to get the SQL pushed on preview.

Comment 31

[Justin Scott [:fligtar]]

Thanks, it looks good. I think the new blur code might be causing a JS problem though.

STR:
1. Expand first category.
2. Click on category again to collapse it.
3. Category collapses and throws JS error.
4. Clicking on any other category won't work now.

Comment 32

[Fred Wenzel [:wenzel]]

(In reply to comment #31)
> Thanks, it looks good. I think the new blur code might be causing a JS problem
> though.

Thanks, rdoherty pointed that out as well. I am in the process of fixing that. Sorry about that.

Comment 33

[Fred Wenzel [:wenzel]]

Fixed: r19225.

Comment 34

[Fred Wenzel [:wenzel]]

Attachment: Trying to save some space

I moved the "I want this" box to the top, and reduced margins. The reason I am asking for review is that when you click on "I want this" now, the appearing "done selecting?" box will result in the description text on the left to be re-flowed a few times. If that looks too weird, we need to look for an alternative.

Comment 35

[Ryan Doherty (:rdoherty)]

Comment on attachment 344198 [details] [diff] [review]
Trying to save some space

I couldn't apply the first part of the patch here (collections_interactive_addon.thtml), but the CSS changes look fine. Height seems a little shorter and still looks good.

Comment 36

[Fred Wenzel [:wenzel]]

r19422.

The first part of the patch moved the install button to the top, but that won't work anymore since that's where you put the "added..." time, so let's keep it where it is for now. Also, the latest wireframes put it in the current spot anyway.

Comment 37

[Fred Wenzel [:wenzel]]

This is implemented so far: Individual bugs to finish it up have been filed, so there's nothing left to do for this bug. Marking FIXED.

Comment 38

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

Verified; individual bugs are indeed the way to go.