590175 - Final spit and polish work for the list view

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Dave Townsend [:mossop]]

Bug 585950 implemented the final list view however we are still waiting on final colours and gradients from UX. This bug will implement those for all platforms.

Comment 1

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Detail views and list view do not line up perfectly for me. Page content shifts up or down a few pixels when changing between the two views.
Should this get its own bug?

Comment 2

[Josh Tumath]

(In reply to comment #1)
> Detail views and list view do not line up perfectly for me. Page content shifts
> up or down a few pixels when changing between the two views.
> Should this get its own bug?

It happens when the label that says "Extensions/Themes/etc..." changes to "<-- Back to extensions".

In the mockups, that label doesn't exist, and is replaced by back/forward buttons. It might be good to file a bug that adds those buttons.

Comment 3

[Dave Townsend [:mossop]]

(In reply to comment #1)
> Detail views and list view do not line up perfectly for me. Page content shifts
> up or down a few pixels when changing between the two views.
> Should this get its own bug?

Meant to correct that before but missed it. It should actually be fixed by bug 562797 where the text headings will be replaced with the forward/back buttons.

Comment 4

[Aaron Train [:aaronmt]]

Get Add-Ons, and it's content pane should be linked without the curved outline on the top, see screenshot http://imgur.com/nYgns.png

Comment 5

[Michael Monreal [:micmon]]

Attachment: Linux issues

Linux issues:
- update button still has mac (round button) look and overlays the drop arrow with a second gear icon
- the (i) icon show in the "undo bar" when removing extensions should use the native icon "gtk-info"
- end of the scrollbars looks broken

General issues:
- the "compatibility" button added by the compatibility reporter extension is shifted one pixel to the bottom.
- why does "undo?" have the question mark?
- why are "restart now" and "undo?" links? that makes no sense
- the search field should align with the boder of the list view
- "Get Addons" should be a few pixels up to avoid the rounded border in the list view

Comment 6

[Alfred Kayser]

Indeed don't use links for actions, buttons are for actions.
Links are to open a page, preferably using a URL.

The restart button doesn't have any id or anonid, it used to have one
        <xul:button class="button-link"
                    label="&addon.restartNow.label;"
                    oncommand="document.getBindingParent(this).restart();"/>
Please add: anonid="restart-btn" to it. (it used to have this).

This is needed to add an icon to the button.

Comment 7

[Andreas Nilsson (:andreasn)]

Attachment: Linux icon for the utilities button

Here is the missing graphics.

Comment 8

[Andreas Nilsson (:andreasn)]

* .view-header could probably use a -moz-appearance: toolbar;
* and the buttons in the header -moz-appearance: button; (but they seems to become awfully big then, maybe make the text a bit smaller)
* It would be cool if we could apply a texture. It works if you set the following things to #addons-page {-moz-appearance: none; background-color: window; background-image: url('name-of-texture.png');} and have that texture be a semi-trasparent pattern of some sorts.

It sounds like I should do a patch. :)

Comment 9

[Andreas Nilsson (:andreasn)]

Attachment: and here is how it would look like (screenshot)

Comment 10

[Andreas Nilsson (:andreasn)]

Attachment: utilities button patch

For some reason it still shows two gear icons (no idea what's going on), but this adds a new icon and makes the button look like a native button.

Comment 11

[Dave Townsend [:mossop]]

(In reply to comment #10)
> Created attachment 470456 [details] [diff] [review]
> utilities button patch
> 
> For some reason it still shows two gear icons (no idea what's going on), but
> this adds a new icon and makes the button look like a native button.

Could you post this in bug 584006 please?

Comment 12

[Andreas Nilsson (:andreasn)]

Attachment: bugfix for the list view header

Gives the list header a toolbar appearance and the border around everything threedshadow. Also takes away the border radius, as that seems to heavy clash with the toolbar visuals (odd clipping going on).

Comment 13

[Spider]

Some comments were invited from the Gnome usability mailing list and I was directed to post mine here.

This is from my point of view as a Gnome platform user, and since opinions are free,  you're also free to ignore them.

---
.01
The main list "Name" and "Last Updated" should probably be a "real" list
(compare to a mail client/file manager) to indicate sorting, with the
arrow up/down for sorting rather than shading the background.

.02
The tab selections look very out of place, not to mention that the top
left tab isn't even aligning to the display part, but cause a jarring
gap between them.

.03
The search field isn't visually aligned on the right hand, causing the
window to look out of proportion once again.

.04
Left/Right alignment of the icon in the search field appears
inconsistent with other applications in the platform ( Gnome typically
has a "clear the search field" brush on the right) However it's also internally consistent within the browser, so that may have it's place.

.05
"View recent updates" slowly fades in a new tab ( that may just be a
performance issue) on the right, which also appears "wrong"

.06
The bevel around the main white area doesn't look like anything else on
the platform. Design choice?

.07
Vertical text alignment, aka "where the top of the text is" is different between the tabs,  mostly because that the "Get addons" lack the upper strip of a list.

Comment 14

[jacob.karlsson95]

Attachment: fixes so that the button with the cogwheel on it doesn't get two cogwheels. sorry i didn't upload the whole file(if you're supposed to do that) but i'm new to this :)

fixes so that the button with the cogwheel on it doesn't get two cogwheels. sorry i didn't upload the whole file(if you're supposed to do that) but i'm new to this :)
this will most hopefully don't break anything :P

Comment 15

[Michael Monreal [:micmon]]

(In reply to comment #14)
> fixes so that the button with the cogwheel on it doesn't get two cogwheels.
> sorry i didn't upload the whole file(if you're supposed to do that) but i'm new
> to this :)

You should attach a patch against the very latest code from mercurial. I recommend dropping into IRC and asking there if you are not sure.

Regarding the "cogwheel" issue: are you sure it is still valid? It's fixed on Linux at least for some time.

Comment 16

[jacob.karlsson95]

Comment on attachment 474088 [details] [diff] [review]
fixes so that the button with the cogwheel on it doesn't get two cogwheels. sorry i didn't upload the whole file(if you're supposed to do that) but i'm new to this :)

the fix may need some pixel pushing to make it look better.

Comment 17

[jacob.karlsson95]

(In reply to comment #15)
> (In reply to comment #14)
> > fixes so that the button with the cogwheel on it doesn't get two cogwheels.
> > sorry i didn't upload the whole file(if you're supposed to do that) but i'm new
> > to this :)
> 
> You should attach a patch against the very latest code from mercurial. I
> recommend dropping into IRC and asking there if you are not sure.
> 
> Regarding the "cogwheel" issue: are you sure it is still valid? It's fixed on
> Linux at least for some time.

i'm running the beta. so it may be fixed already :P

Comment 18

[Michael Monreal [:micmon]]

(In reply to comment #17)
> i'm running the beta. so it may be fixed already :P

Best if you attach a small before/after screenshot and mention the OS you are running on.

(work on the next beta already started about 2 weeks ago, so to be current you should test nightly builds at least. for providing patches, check out the code from the mercurial repository)

Comment 19

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #17)
> i'm running the beta. so it may be fixed already :P

Yes, fixed by the first patch in bug 584006.


Also: Could people attach their patches to more specific bugs that are dependent on this bug, please. Even if that means filing a new bug just for one tiny little tweak. Having many different patches for many different things in this one bug is going to make it very difficult to keep track of, and I don't want any of your good work lost in a sea of other patches (or anyone duplicating work that's already been done). Thanks!

Comment 20

[Dave Townsend [:mossop]]

Attachment: List view spec OSX

Comment 21

[Dave Townsend [:mossop]]

Attachment: List view spec Windows

Comment 22

[Dave Townsend [:mossop]]

Attachment: Notifications spec OSX and Windows

Comment 23

[Dave Townsend [:mossop]]

Attachment: List view resources

Comment 24

[Dave Townsend [:mossop]]

Attachment: Surrounding view resources

Comment 25

[Tim Meader]

Just wondering, are there any plans to at also dim the text of disabled add-ons (to further note an "off" state)? I know in the original mock-up in bug 585950 (https://bug585950.bugzilla.mozilla.org/attachment.cgi?id=464424), the disabled add-ons show with a different background gradient as well as a dimmed font. Until the new gradients are landed, would it be possible to at least dim the font as in the proposal case? Just some form of differentiation when looking at the list view besides "(disabled)" would be very helpful for quickly identifying those add-ons visually without having to actively read each extension name to see the "(disabled)" on the end. Apologies if this was waiting for some other reason.

Comment 26

[Dave Townsend [:mossop]]

(In reply to comment #25)
> Just wondering, are there any plans to at also dim the text of disabled add-ons
> (to further note an "off" state)? I know in the original mock-up in bug 585950
> (https://bug585950.bugzilla.mozilla.org/attachment.cgi?id=464424), the disabled
> add-ons show with a different background gradient as well as a dimmed font.
> Until the new gradients are landed, would it be possible to at least dim the
> font as in the proposal case? Just some form of differentiation when looking at
> the list view besides "(disabled)" would be very helpful for quickly
> identifying those add-ons visually without having to actively read each
> extension name to see the "(disabled)" on the end. Apologies if this was
> waiting for some other reason.

The spec for this bug that I attached in comments 20 and 21 show the disabled states dimmed.

Comment 27

[Dave Townsend [:mossop]]

Fixed by bug 601022, file any remaining issues separately

Comment 28

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Marking as verified with builds on all platforms like Mozilla/5.0 (X11; Linux
i686; rv:2.0b8pre) Gecko/20101125 Firefox/4.0b8pre ID:20101125030318

Blair, I assume those were only CSS updates, so no test is required especially for this type of list.

Comment 29

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #28)
> Blair, I assume those were only CSS updates, so no test is required especially
> for this type of list.

There was a couple of JS changes too, but they were specific to the search view, and have automated tests. One was bug 579636, the other was setting "first"/"last" attributes on the first/last visible items in the search view (no specific bug for that). Both are tested in toolkit/mozapps/extensions/test/browser/browser_searching.js