324.28 KB, image/png
8.28 KB, application/zip
210.02 KB, image/png
14.68 KB, patch
|Details | Diff | Splinter Review|
63.10 KB, image/png
Need to add a slider to rotate through recommended add-ons on home page.
Created attachment 321136 [details] slider mockup, rev 1 This is the kind of thing I was thinking. Thoughts?
Yes, can we make it cleaner by removing the 1-5 buttons? This way you don't have to worry about which one you are on, having to mark the selection of the button, dealing with going left when you are on 1, etc... See http://www.comcast.com/ for an example of < " > [left, pause, right]. This will allow us to auto-scroll and give users the ability to pause it. Do you like that?
Created attachment 321869 [details] 3 slider variants Basil - so you're thinking something more like (a) in the attached mockup? Are you sure we want these to cycle automatically?
Even though I suggested it (auto-scrolling), I think we should start with Option C in the attachment for comment 3. If you feel strongly about auto-cycling thru the choices, then we go with option A.
Apple uses dots (a filled dot being the current "page", all others being circles), if I remember correctly. Not sure if that would make it any better than option C, though.
Let's use C, since it allows us to grow arbitrarily from 5 over time.
Created attachment 327191 [details] [diff] [review] Initial stab at a slider on the home page Okay, so here's an initial stab at implementing a slider with multiple recommended addons. I'm thinking this patch from svn diff may not quite work directly, since there are images involved - not sure about that. I've got this running here on khan internally: http://amo.lorchard.khan.mozilla.org/en-US/firefox/ And viewing this may require the following /etc/hosts addition: 10.2.74.111 amo.lorchard.khan.mozilla.org There's a bit of extra spacing in the slider rendering of addons, but that's mostly to try to account for various sizes of titles, author names, etc in one consistently-sized slider viewport.
Adding stephend, since he'll probably want to see this
Les, this is looking really good. Is the rapid animation at wrap-around (From 5->1 or 1->5) optional? If so, I personally find it distracting. Can we do a smooth slide like the other transitions?
For longer text descriptions, I'm seeing vertical scrollbars; if we fixed our site-wide CSS model to use :float, would that no longer be a problem (the page's height would automatically adjust to fit all content)?
Yeah, the scrollbar is a bit of a compromise that might need some playing around with. The problem with allowing the height to adjust automatically is that all the different lengths of description text for that section causes the prev/next controls at the bottom to jump up and down with each click.
Created attachment 327831 [details] [diff] [review] Revised slider with variable height Okay, I here's a reworked slider that should expand vertically to accomodate the longest addon item in the set. Also, I've switched from "1 of 5" to "1 / 5" because I'm a bit unsure how "X of Y" would translate into various languages. This update should be available on my khan domain now: http://amo.lorchard.khan.mozilla.org/en-US/firefox/
Just saw baz's comment about the start and end animations. That'll take some thinking: For this slider, the addons are laid out on a wide horizontal strip side-by-side with a viewport revealing one addon width. Seemed like the easiest way to do it. There's not really a good way to continuously or smoothly go from one end of the strip to the other. If it's too distracting, I could disable previous for 1/5 and next for 5/5.
As I told Les in IRC, this looks good to me, sans the slightly jarring "whirring" issue Basil noted in comment 9 -- are we ready for reviews yet, or do we want to implement Les's proposed mitigation in comment 13?
fwiw, I like the fast motion going from one end to the other as a visual cue that I did something different than the previous 4 clicks.
My recommendation is for pressing the left arrow at 1 to create another small left-scroll to 5, and similarly for pressing the right arrow at 5 to create another right-scroll to 1. In this way, pressing one arrow over and over makes the entire set cycle with no breaks at either end. The reason I don't think maintaining a solid representation of the slides going from 1-5 is important is because the slides are completely unrelated to each other; there's no progression or reason to limit the user's actions by their order. I agree that the fast scrolling is very distracting. The user first pressing the left arrow would likely be expecting another add-on to appear: for it to do so without a transition would likely be expected, for it to do so with a transition would be a good surprise ("cool!" - person sitting next to me), and for it to quickly blow past three other options is a negative surprise ("what happened? can I go back?"). I don't think disabling the buttons at either end is better either. The affordance to begin viewing is clearly here with the arrows, and for one to not work would just seem broken. Even to gray out the arrow would be an unnecessary restriction on the user, since there's no importance of viewing the slides 1-5 rather than 1, 5, 4, 3, 2.
Basically, with this current implementation, the options are to disable the buttons at either edge or to leave the fast scrolling enabled. Basically right now it's a wide strip of addons statically laid out with a moving viewport window sliding over it. To have a continuous transition from 5 to 1 and vice versa, I'll need to go back to the drawing board and find another way to stitch the addon items together. The conceptual alternative would be much more complex and dynamic: Attempting to actively position the next addon in the desired direction just before the transition fires. Not entirely sure how I'd go about that, off the top of my head - thus, the drawing board and probably not ready by Monday. :)
Sure, makes sense. Would it be possible to render a disabled version of the button at either end?
I honestly don't mind the way it is right now. On the other hand, clicking to the right, and achieving a "going left" is not good for stimulus-response compatibility. I do think we need a way to jump all the way back though, and arrows along with a "1/5" display may not be the best way to do it. How about <- o o o O o -> where the big O is a filled circle, and the lowercase 'o's are rings? In fact, I just found this pattern on the Yahoo design pattern library as well: http://developer.yahoo.com/ypatterns/pattern.php?pattern=carousel
boriss: I cut the arrows from the Madhava-supplied mockup. I think I'd need either him or someone with the original photoshop doc to render them. My graphic-design-fu is not strong. wenzel: Yup, I've seen that Carousel pattern. I went with the 1/5 indicator per Beltzner in comment #6, because I think it makes it easier to go from 5 to whatever number we want. Though, of course, the other pattern makes random access easier. https://bugzilla.mozilla.org/show_bug.cgi?id=432669#c6 We could go with a different position indicator if need be, though. Clock's a-tickin' though :) (And hey, I got through this comment without once writing "basically"... Oh. Crud. There it goes.)
l.m.orchard: I can make you some arrows, I mean would it be possible to have an active arrow that displays if the user can press it and switch it to a deactivated, faded-out version for the left arrow at 1 and right arrow at 5? This might be a good way to go since it would never disappoint the user.
boriss: Oh, sure - given the images, I can swap when one of the buttons needs to be disabled.
Arrows with a deactivated state may be a good temporary fix since the clock's a-tickin (see attachement) Wenzel - very cool link, perhaps we can do something along those lines when there's more time to devote to a solution
Created attachment 328376 [details] [diff] [review] Revised slider with conditionally disabled prev/next buttons Okay, I've updated my staging host with the disabled buttons: http://amo.lorchard.khan.mozilla.org/en-US/firefox/
Comment on attachment 328376 [details] [diff] [review] Revised slider with conditionally disabled prev/next buttons r? :roherty
Created attachment 328395 [details] OSX screenshot with overlap from Recommended column l.m.orchard: Looks great, although there's seems to be an overlap problem when the window's width is shortened on OSX (see screenshot).
Comment on attachment 328376 [details] [diff] [review] Revised slider with conditionally disabled prev/next buttons Works, but could use some improvement :) -Doesn't handle browser window smaller than 1046px wide -Each addon could be in a list instead of divs. -Doesn't work without JS. Buttons could just be links to the homepage and have ?slider_index=2,3, etc. -Can the JS slider code be a jQuery plugin? Then we can use it anywhere and just have 1 line of JS at the bottom of the page initializing it. -The slider code should be moved to addons.js to gain caching, minification, etc. -Instead of testing the target that is clicked, there could be id's added to the buttons and the id could be used to figure out if the next/prev button was clicked. -Some sort of internal state could be used to track what addon is shown. Then it can know if there's a next or previous addon to display and I think some logic and css classnames could be removed. Less DOM manipulation to save state, the better. -Some browser history management would be nice too (URL#slider-index=2). That way people who click the next arrow can send a link to a friend that would show the same addon.
Okay... that list won't be done for code freeze tonight.
Created attachment 328441 [details] [diff] [review] Slider rewritten with resizability and jQuery plugin-ability Okay, this patch doesn't address everything, but: - Browser resizing should be handled now - Switched to list items from divs - Still doesn't work without JS - It's now rewritten from scratch as a jQuery plugin, fired up with one line of code near the bottom of the page. - The plugin is in addons.js - The next / prev buttons are given event handlers directly now. It was done using event delegation because a previous version gave each addon its own next / prev button, but the single control set obviates that. - An internal index is used to select through a list of items fetched at startup. - Browser history isn't useful for this bug, because the addons are selected at random for each page load. Could be a future addition to the plugin though. going to bed now.
Comment on attachment 328441 [details] [diff] [review] Slider rewritten with resizability and jQuery plugin-ability Looks really good. r+ it for code freeze, but would like to have the few small improvements done during the next release.
Just checked this in as r16796
On https://preview.addons.mozilla.org/en-US/firefox/ Verified Fixed -- Slider is there and resizing browser window is handled well.
Created attachment 328906 [details] Lots of empty space Just noticed a small issue where if 1 addon has a large description, it causes the slider height to be extremely tall. This causes other addons listed to have lots of empty space. Could we trim all descriptions to a max # of characters to prevent this?
(In reply to comment #34) > Could we trim all descriptions to a max # of characters to prevent this? See bug 443977 for that. We could also consider moving the button and "view more" link to the left side to give more room for the description on the right.