Last Comment Bug 309538 - Autoscroll indicator should show possible scrolling directions
: Autoscroll indicator should show possible scrolling directions
Status: RESOLVED FIXED
: fixed1.8
Product: SeaMonkey
Classification: Client Software
Component: UI Design (show other bugs)
: Trunk
: x86 Windows XP
: -- normal (vote)
: ---
Assigned To: Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com]
:
Mentors:
Depends on: 304563
Blocks: 280331
  Show dependency treegraph
 
Reported: 2005-09-21 14:26 PDT by Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com]
Modified: 2008-07-31 04:22 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (8.26 KB, patch)
2005-09-21 14:51 PDT, Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com]
neil: superreview-
Details | Diff | Review
north-south icon (927 bytes, image/gif)
2005-09-21 14:52 PDT, Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com]
no flags Details
east-west icon (941 bytes, image/gif)
2005-09-21 14:53 PDT, Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com]
no flags Details
north-south icon (optimized gif) (171 bytes, image/gif)
2005-09-21 18:47 PDT, Gérard Talbot
no flags Details
east-west icon (optimized gif) (183 bytes, image/gif)
2005-09-21 18:52 PDT, Gérard Talbot
no flags Details
patch v2 (8.10 KB, patch)
2005-10-14 21:19 PDT, Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com]
db48x: review+
neil: superreview+
asa: approval1.8rc1+
Details | Diff | Review
what I checked in on trunk (7.97 KB, patch)
2005-10-20 20:43 PDT, Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com]
no flags Details | Diff | Review

Description Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2005-09-21 14:26:44 PDT
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 cst@andrew.cmu.edu cst@yecc.com] 2005-09-21 14:51:28 PDT
Created attachment 196948 [details] [diff] [review]
patch
Comment 2 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2005-09-21 14:52:33 PDT
Created attachment 196949 [details]
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 cst@andrew.cmu.edu cst@yecc.com] 2005-09-21 14:53:14 PDT
Created attachment 196950 [details]
east-west icon

Same deal here.
Comment 4 Gérard Talbot 2005-09-21 18:47:40 PDT
Created attachment 196987 [details]
north-south icon (optimized gif)
Comment 5 Gérard Talbot 2005-09-21 18:52:12 PDT
Created attachment 196988 [details]
east-west icon (optimized gif)
Comment 6 neil@parkwaycc.co.uk 2005-09-26 06:44:14 PDT
256 colours is overkill, but I can actually optimize them down to 4 colours ;-)
Comment 7 neil@parkwaycc.co.uk 2005-09-26 09:14:57 PDT
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 cst@andrew.cmu.edu cst@yecc.com] 2005-10-14 21:19:58 PDT
Created attachment 199628 [details] [diff] [review]
patch v2

I left the "NSEW" attribute value for multi-directional scrolling.
Comment 9 neil@parkwaycc.co.uk 2005-10-15 15:51:56 PDT
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 cst@andrew.cmu.edu cst@yecc.com] 2005-10-20 20:19:27 PDT
Comment on attachment 199628 [details] [diff] [review]
patch v2

SeaMonkey-only patch
Comment 11 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2005-10-20 20:43:16 PDT
Created attachment 200312 [details] [diff] [review]
what I checked in on trunk

Note You need to log in before you can comment on or make changes to this bug.