423469 - Theme list can be completely obscured

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Madhava Enros [:madhava]]

Attachment: theme list obscured (preview pane full width)

See attached image.  The preview pane (right side) can be expanded to fill the whole area, completely obscuring the list of themes.  There should probably be a minimum width set for the themes list so that users can't get into this state.

Comment 1

[mcdavis941 (sporadically reading bugmail)]

See also related Bug 387999, for when you move the slider the other way.

Comment 2

[Dave Townsend [:mossop]]

Just needs an appropriate min-width setting

Comment 3

[Nochum Sossonko [:Natch]]

Attachment: For winstripe, pinstripe, and gnomestripe

Used 230px as the min-width on extensionView, which changes it for addons, themes, plugins, etc.

Also, from the OS I deduced that this needs to be changed on all three?

Comment 4

[Nochum Sossonko [:Natch]]

Attachment: Patch for this and bug 423469

I don't know if this is what's wanted, but here's a patch for both bugs, (and consequently the one about the splitter), but it seems that the collaps=after was intentional?

Comment 5

[Dave Townsend [:mossop]]

Are both of these patches necessary?

Comment 6

[Nochum Sossonko [:Natch]]

Attachment: New patch, same thing

New one just takes away width attribute... not necessary since there's a min-width set in css.

(in reply to comment #5)

No, sorry just forgot to obsolesce the other. here's a new one though

Comment 7

[Dave Townsend [:mossop]]

Comment on attachment 336360 [details] [diff] [review]
New patch, same thing

The problem with this patch is that it seems to allow the size of the theme preview area to grow beyond the bounds of the window, causing a horizontal scrollbar for the entire dialog which isn't good.

If the preview is too large we should get a scrollbar in the preview area I think, not for the entire window, though I think there is something going on with long text lines no longer wrapping because you've removed the fixed width from the area

Comment 8

[Dave Townsend [:mossop]]

Attachment: OSX screenshot

This is a screenshot of the default add-ons manager window on OSX with the existing patch applied.

Comment 9

[Dave Townsend [:mossop]]

Attachment: Linux screenshot

Linux goes for the double whammy of two scrollbars

Comment 10

[Nochum Sossonko [:Natch]]

(In reply to comment #9)
> Created an attachment (id=336456) [details]
> Linux screenshot
> 
> Linux goes for the double whammy of two scrollbars

This is most probably because of the min-width not being big enough to fit the elements on Linux... easy fix although the themePreview scrolling issue is still a problem... I'll hopefully get to it some time later today.

Comment 11

[Nochum Sossonko [:Natch]]

Attachment: Some improvement

This patch proposes the following:

The min-widths put scrollbars in mind (i.e. an extra 5px) so that when the window is too short to show the elements, but still wide enough, only vertical scrollbars show. Resizing the window resizes both the extensionsView and previewImage to their min-widths. If the window is too narrow, one scrollbar shows on the bottmom. Now I added some width on the Linux version, but I don't have a Linux box or a Mac to test either platforms.

(note: IMO the extensionsView box should have a flex=4 or something as it makes more sense to stretch that than to stretch the previewImage, but I left it with flex=1).

Comment 12

[Dave Townsend [:mossop]]

I'm thinking this might be as good as we can get it. There is still an issue with a full width scrollbar if you shrink the add-ons manager down enough that you don't get with the current set-up, if we could avoid that then we've won.

Comment 13

[Nochum Sossonko [:Natch]]

Attachment: Addresses comment 12

I didn't marks the older patch obsolete b/c this one proposes a different style entirely, so I'll leave it up to you to chose which one would be better.

With this the scrolls show up individually for each box, extensionsView and previewImage. However, the min-widths were reduced so that the splitter can be dragged over the boxes albeit not all the way.

I think this is the only way to achieve individual scrollbars, being that any box with a min-width will never overflow unless the min-width is too small to hold its own contents (rather the parent will overflow, causing the window-wide scrollbars), but on the other hand the splitter can only be stopped by min-widths on the elements directly before and after it.

The two remaining patches are either/or.

Comment 14

[Nochum Sossonko [:Natch]]

Attachment: Same thing fixes small mistake

Sorry about the spam, this just evens out the previewImageDeck width on Mac with the other systems.

Comment 15

[Nochum Sossonko [:Natch]]

Shouldn't this be dependent on bug 431023, after all that just might effect some dimension decisions here as well?

Comment 16

[Dave Townsend [:mossop]]

Attachment: patch for checkin

I like the latter approach better, especially since it fixes bug 387999 as well, so I'll land that. I've just made a minor change and increased the min-width on extensionsView back to 245px.

Comment 17

[Dave Townsend [:mossop]]

Landed: http://hg.mozilla.org/mozilla-central/rev/76d6c5e513ec

Comment 18

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

Verified with:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b1) Gecko/20081007 Firefox/3.1b1 ID:20081007125523

Mozilla/5.0 (Windows; U; Windows NT 6.0; de; rv:1.9.1b2pre) Gecko/20081016 Minefield/3.1b2pre

Comment 19

[francisdinh]

The opposite of this bug is bug# 487352.