615476 - Arrowpanel arrows should be inset more

Status: RESOLVED WONTFIX

(Toolkit :: Themes, defect)

Description

[Alex Faaborg [:faaborg] (Firefox UX)]

Currently the arrow on arrow panels nearly touches the edge.  We should inset this more (about 28 pixels, or whatever the width is of the current arrow).  Also for notification arrow panels we should make sure that the 64x64 icon is centered to the peak of the arrow.

Comment 1

[Alex Faaborg [:faaborg] (Firefox UX)]

Attachment: Inset arrow example at 8x

Comment 2

[:Margaret Leibovic]

Attachment: patch

This patch insets the arrow more when it is on the top/bottom for all arrow panels.

I based the margin values on centering the doorhanger icon under the arrow. Alex, do we want this much of an inset for all arrow panels, or should I make these margins doorhanger-specific? Also, do we also want the arrow to be inset more when it's on the left/right?

Comment 3

[:Margaret Leibovic]

Attachment: doorhanger notification arrows (aero, xp, osx)

Centering the arrow under the small icon and the large icon under the arrow is impossible because the icons have even widths and the arrow ends in a 1px point. I opted for pointing the arrow to the left side of center of the small icon, and I aligned the arrow so that the large icon is centered under the small icon (at least that's what I think I did -- I may have been staring at pixels for too long and made a mistake).

Comment 4

[Dão Gottwald [:dao]]

Comment on attachment 496580 [details] [diff] [review]
patch

Will this patch move the panel content off-screen when the anchor is near the screen edge?

Comment 5

[:Margaret Leibovic]

(In reply to comment #4)
> Comment on attachment 496580 [details] [diff] [review]
> patch
> 
> Will this patch move the panel content off-screen when the anchor is near the
> screen edge?

Yes, that's what it's doing. However, I'm getting a "Popup is offscreen" assertion from my debug build that I need to ignore before the panel will render. Is that okay?

Comment 6

[Dão Gottwald [:dao]]

I don't know about the assertion, but from a user point of view this doesn't seem okay...

Comment 7

[:Margaret Leibovic]

How often will the popup open that close to the edge of the screen? In that case, browser content is probably off-screen as well, so would the user really be upset by off-screen popup content?

Also, this already happens, so seems like this patch just makes an existing issue more noticeable. And I don't know if there is a good solution for the problem, since the anchor itself can even be off-screen.

Comment 8

[Dão Gottwald [:dao]]

No part of the window needs to be off-screen for that. Just make the url bar the first item in the nav bar, e.g. by moving back/forward to the right or up to the tab bar.

Comment 9

[:Margaret Leibovic]

Alex, what do you think about this off-screen issue. I agree with Dão that it would be bad to have the panel appear off-screen if the window is completely on-screen, especially if it is maximized. I would suggest we move the arrow over in that case, but that will make this patch more complicated.

Comment 10

[Alex Faaborg [:faaborg] (Firefox UX)]

Moving the arrow in the case of being really close to a screen edge is fine (if moving it allows it to fit).  But if there is plenty of room, we should go for the more visually appealing arrow placement.

Comment 11

[Neil Deakin]

(In reply to comment #5)
> Yes, that's what it's doing. However, I'm getting a "Popup is offscreen"
> assertion from my debug build that I need to ignore before the panel will
> render. Is that okay?

No, it isn't. That's bug 524545, which I will look into fixing.

Comment 12

[Neil Deakin]

I updated the patch in that bug. Could you see if it fixes the issue for you?

Comment 13

[:Margaret Leibovic]

Enn's patch prevents the panel from appearing off screen, but the arrow points to the wrong place if the panel is at the edge of the screen, and if the anchor is completely off screen, the arrow disappears.

I'll need to change this patch to have the arrow move over as the anchor gets closer to the edge of the screen, but at some point it won't be able to point to center of the anchor even when the anchor is still on screen, due to the width of the arrow. However, I think this is small enough of an edge case that it's not a big deal.

Comment 14

[Neil Deakin]

Comment on attachment 496580 [details] [diff] [review]
patch

On Windows XP (non-aero) the difference that you adjust the panel margin and the arrow margin is one pixel different. On other platforms you adjust both by the same amount. Was this intentional?

Comment 15

[:Margaret Leibovic]

Yeah, I did that intentionally to get the arrow to align with the pixel to the left of the center of the anchor icon. Right now the arrows on XP point to the right side of center of the anchor icon, which is different than aero and OSX.

Comment 16

[Dão Gottwald [:dao]]

(In reply to comment #13)
> I'll need to change this patch to have the arrow move over as the anchor gets
> closer to the edge of the screen, but at some point it won't be able to point
> to center of the anchor even when the anchor is still on screen, due to the
> width of the arrow. However, I think this is small enough of an edge case that
> it's not a big deal.

It's a big relatively deal as you're affecting arrow panels in general here (e.g. those used for form validation, or those that widgets in the add-on bar might want to use), so it's going to be quite easy to trigger these situations.

Comment 17

[:Margaret Leibovic]

Well, that's a case that exists right now, so basically I would be moving the arrow over if possible, and defaulting to the current positioning if there isn't room to move it over.

It's still a problem that currently exists, but I think the solution is out of the scope of this bug. Fixing it would require us to figure out what to do if the anchor icon is at the edge of the screen, and I don't know what that would look like. Maybe something like putting the arrow at the corner of the popup. However, I think that's probably out of the scope of Firefox 4.

Comment 18

[u428464]

This would be a first good move towards the Australis arrow panels.

Comment 19

[Jared Wein [:jaws] (please needinfo? me)]

We now move the arrow when the panel has flip="slide" so the issue that Margaret was running in to (comment #16) may not be an issue anymore.

Comment 20

[u428464]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #19)
> We now move the arrow when the panel has flip="slide" so the issue that
> Margaret was running in to (comment #16) may not be an issue anymore.

Isn't it only the fact for the menu panel ?

Comment 22

[Tim Nguyen :ntim]

Can this be closed ?