Closed Bug 309538 Opened 16 years ago Closed 16 years ago

Autoscroll indicator should show possible scrolling directions

Categories

(SeaMonkey :: UI Design, defect)

x86
Windows XP
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: csthomas, Assigned: csthomas)

References

(Blocks 1 open bug)

Details

(Keywords: fixed1.8)

Attachments

(6 files, 1 obsolete file)

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.
Status: NEW → ASSIGNED
Attached patch patch (obsolete) — Splinter Review
Attachment #196948 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #196948 - Flags: review?(db48x)
Attached image north-south icon
This needs to be optimized.  I just cut some arrows out of the existing icon,
and the file got much bigger.
Attached image east-west icon
Same deal here.
256 colours is overkill, but I can actually optimize them down to 4 colours ;-)
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
Attachment #196948 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview-
Component: General → XP Apps: GUI Features
Blocks: 280331
Whiteboard: [cst:r?[
Attachment #196948 - Attachment is obsolete: true
Attachment #196948 - Flags: review?(db48x)
Attachment #196987 - Attachment filename: autoscrollNS2.gif → autoscroll-ns.gif
Attachment #196988 - Attachment filename: autoscrollEW2.gif → autoscroll-ew.gif
Attached patch patch v2Splinter Review
I left the "NSEW" attribute value for multi-directional scrolling.
Attachment #199628 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #199628 - Flags: review?(db48x)
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?
Attachment #199628 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
Attachment #199628 - Flags: review?(db48x) → review+
Comment on attachment 199628 [details] [diff] [review]
patch v2

SeaMonkey-only patch
Attachment #199628 - Flags: approval1.8rc1?
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Whiteboard: [cst:r?[ → [cst: a?] [cst: fixed-trunk]
Attachment #199628 - Flags: approval1.8rc1? → approval1.8rc1+
Keywords: fixed1.8
Whiteboard: [cst: a?] [cst: fixed-trunk]
Component: XP Apps: GUI Features → UI Design
You need to log in before you can comment on or make changes to this bug.