309538 - Autoscroll indicator should show possible scrolling directions

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

The indicator that appears when you start autoscrolling should show what
scrolling directions are possible.  In a normal page, for example, there should
only be north-south arrows.  A wide but short page should result in east-west
arrows.  Attachment 184934 [details] [diff] on bug 296080 has some code that could be useful. 
Given how we set the icon, though, we'd have to set an attribute on the
autoscroller which gets matched by the CSS.

Comment 1

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: patch

Comment 2

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: north-south icon

This needs to be optimized.  I just cut some arrows out of the existing icon,
and the file got much bigger.

Comment 3

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: east-west icon

Same deal here.

Comment 4

[Gérard Talbot]

Attachment: north-south icon (optimized gif)

Comment 5

[Gérard Talbot]

Attachment: east-west icon (optimized gif)

Comment 6

[[e-mail]]

256 colours is overkill, but I can actually optimize them down to 4 colours ;-)

Comment 7

[[e-mail]]

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

>+  skin/classic/communicator/icons/autoscrollNS.gif                        (communicator/icons/autoscrollNS.gif)
>+  skin/classic/communicator/icons/autoscrollEW.gif                        (communicator/icons/autoscrollEW.gif)
Should be autoscroll-ns or -ew. (Or possibly ns- or ew-autoscroll, compare
ns-resize cursor).

>+#autoscroller[scrolldir="NSEW"] {
>   background-image: url("chrome://communicator/skin/icons/autoscroll.gif");
> }
>+#autoscroller[scrolldir="NS"] {
>+  background-image: url("chrome://communicator/skin/icons/autoscrollNS.gif");
>+}
>+#autoscroller[scrolldir="EW"] {
>+  background-image: url("chrome://communicator/skin/icons/autoscrollEW.gif");
>+}
Please choose one rule to be default i.e. not using an attribute selector.

>+  var scrollingDir = "";
>+  if (gScrollingView.scrollMaxY > 0) {
>+    scrollingDir += "NS";
>+  }
>+  if (gScrollingView.scrollMaxX > 0) {
>+    scrollingDir += "EW";
>+  }
>+  if (!scrollingDir)
>+    scrollingDir = "NSEW";
You should probably do nothing (abort the scroll) if the frame isn't
scrollable. If you make ns the default then maybe you could write it like this:
if (gScrollingView.scrollMaxX > 0)
  gAutoScrollPopup.setAttribute("scrolldir", gScrollingView.scrollMaxY > 0 ?
"nsew" : "ew")
else if (gScrollingView.scrollMaxY > 0)
  gAutoScrollPopup.removeAttribute("scrolldir"); // default
else
  return; // don't scroll

Comment 8

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: patch v2

I left the "NSEW" attribute value for multi-directional scrolling.

Comment 9

[[e-mail]]

Comment on attachment 199628 [details] [diff] [review]
patch v2

Remind me to get you those fully optimized images (137/148 bytes).

> #autoscroller { 
>   height: 28px;
>   width: 28px;
>   border: 0px;
>   margin: -14px;
>   padding: 0px;
>+}
>+#autoscroller {
Unnecessary addition (applies to both themes).

>+    gScrollingView = null; // make sure we don't think we're scrolling
Wouldn't "abort scrolling" suffice?

Comment 10

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Comment on attachment 199628 [details] [diff] [review]
patch v2

SeaMonkey-only patch

Comment 11

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: what I checked in on trunk