Closed
Bug 342841
Opened 19 years ago
Closed 19 years ago
Scrolling arrow should look disabled when you can't scroll in its direction
Categories
(Toolkit :: XUL Widgets, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla1.8.1beta1
People
(Reporter: ancestor.ak, Assigned: asaf)
References
(Blocks 1 open bug)
Details
(Keywords: fixed1.8.1, Whiteboard: 181b1+)
Attachments
(4 files, 3 obsolete files)
(deleted),
patch
|
mtschrep
:
approval1.8.1+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
mconnor
:
first-review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mconnor
:
first-review+
|
Details | Diff | Splinter Review |
Scrolling arrows introduced by patch in bug 318168 fail to indicate that you have reached the end of the tab bar and you can't scroll any further in that direction. As a result, user can't tell (i.e. has no visual indication) where he is in the tab bar.
Assignee | ||
Updated•19 years ago
|
Flags: blocking-firefox2?
Comment 1•19 years ago
|
||
Disabled appearance > hidden, since otherwise there's annoying jumps.
Not a blocker, will take patch.
Severity: normal → trivial
Flags: blocking-firefox2? → blocking-firefox2-
Can a :hover state be added to the arrows as well under this bug? Maybe a popup list of the tabs that are not yet visible?
Comment 3•19 years ago
|
||
*** Bug 342907 has been marked as a duplicate of this bug. ***
Comment 4•19 years ago
|
||
adam (and ben) point out this is needed because otherwise, it can be hard to know if you have more tabs (to the left or right)
Assignee: nobody → sspitzer
OS: Windows XP → All
Hardware: PC → All
Comment 5•19 years ago
|
||
This is a blocker I think, probably not for b1 though. Since this is new functionality, and we're going to crow about it, it should work right. Right now it's a bit misleading and has been confusing the people I've seen trying to use it.
Flags: blocking-firefox2- → blocking-firefox2+
Updated•19 years ago
|
Whiteboard: 181b1+
Updated•19 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → Firefox 2 beta1
Comment 6•19 years ago
|
||
in order to prevent the ui from moving out from underneath the user while scrolling (and they hit the edge), mconnor and I agree that disabling is the right fix.
see related bug #222274
Summary: Scrolling arrow should be hidden or look disabled when you can't scroll in its direction → Scrolling arrow should look disabled when you can't scroll in its direction
Assignee | ||
Comment 7•19 years ago
|
||
Taking. I'm attaching the binding changes. Could someone provide disabled arrows images (at least temporary images)?
Assignee: sspitzer → bugs.mano
Severity: trivial → normal
Status: ASSIGNED → NEW
Priority: -- → P2
Assignee | ||
Updated•19 years ago
|
Status: NEW → ASSIGNED
Component: Tabbed Browser → XUL Widgets
Product: Firefox → Toolkit
QA Contact: tabbed.browser → xul.widgets
Target Milestone: Firefox 2 beta1 → mozilla1.8.1beta1
Version: 2.0 Branch → 1.8 Branch
Assignee | ||
Comment 8•19 years ago
|
||
This is just the binding changes.
(side note: this will probably also fix bug 222274 if vertical disabled arrows images are provided as well ;) )
Attachment #227984 -
Flags: second-review?(mconnor)
Attachment #227984 -
Flags: first-review?(sspitzer)
Assignee | ||
Comment 9•19 years ago
|
||
typos fixes
Attachment #227984 -
Attachment is obsolete: true
Attachment #227986 -
Flags: second-review?(mconnor)
Attachment #227986 -
Flags: first-review?(sspitzer)
Attachment #227984 -
Flags: second-review?(mconnor)
Attachment #227984 -
Flags: first-review?(sspitzer)
Comment 10•19 years ago
|
||
Comment on attachment 227986 [details] [diff] [review]
patch (binding changes only)
r=sspitzer
as mentioned on irc, please remove these two lines from the _updateScrollButtonsDisabledState() method:
+ var isLTR = window.getComputedStyle(this.parentNode, "")
+ .direction == "ltr";
Attachment #227986 -
Flags: first-review?(sspitzer) → first-review+
Updated•19 years ago
|
Attachment #227986 -
Flags: second-review?(mconnor) → second-review+
Assignee | ||
Comment 11•19 years ago
|
||
Actually, it looks like we do have disabled arrows images.
Assignee | ||
Comment 12•19 years ago
|
||
mozilla/toolkit/content/widgets/scrollbox.xml 1.11
Attachment #227986 -
Attachment is obsolete: true
Attachment #228022 -
Flags: approval1.8.1?
Assignee | ||
Comment 13•19 years ago
|
||
Comment on attachment 228022 [details] [diff] [review]
patch as checked in (binding changes only)
This can't land on the 1.8 branch without the fix for bug 314350.
Attachment #228022 -
Flags: approval1.8.1?
Assignee | ||
Comment 14•19 years ago
|
||
Attachment #228027 -
Flags: first-review?(mconnor)
Comment 15•19 years ago
|
||
Comment on attachment 228027 [details] [diff] [review]
Winstripe changes
r=me, with the missing piece (dis) in the right places
Attachment #228027 -
Flags: first-review?(mconnor) → first-review+
Assignee | ||
Comment 16•19 years ago
|
||
mozilla/toolkit/themes/winstripe/global/scrollbox.css 1.7
Attachment #228027 -
Attachment is obsolete: true
Assignee | ||
Comment 17•19 years ago
|
||
This makes the above magic work for menus (e.g. bookmark folders).
Attachment #228029 -
Flags: first-review?(mconnor)
Updated•19 years ago
|
Attachment #228029 -
Flags: first-review?(mconnor) → first-review+
Assignee | ||
Comment 18•19 years ago
|
||
Comment on attachment 228029 [details] [diff] [review]
make this work for legacy autorepeat-based scrollboxes (checked in)
mozilla/toolkit/content/widgets/scrollbox.xml 1.12
Attachment #228029 -
Attachment description: make this work for legacy autorepeat-based scrollboxes → make this work for legacy autorepeat-based scrollboxes (checked in)
Assignee | ||
Comment 19•19 years ago
|
||
Attachment #228032 -
Flags: first-review?(mconnor)
Updated•19 years ago
|
Attachment #228032 -
Flags: first-review?(mconnor) → first-review+
Assignee | ||
Comment 20•19 years ago
|
||
Comment on attachment 228032 [details] [diff] [review]
Pinstripe changes (checked in)
mozilla/toolkit/themes/pinstripe/global/scrollbox.css 1.5
Attachment #228032 -
Attachment description: Pinstripe changes → Pinstripe changes (checked in)
Assignee | ||
Updated•19 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Whiteboard: 181b1+ → 181b1+ branch landing is pending approval for 314350 and 343097
Assignee | ||
Comment 21•19 years ago
|
||
Comment on attachment 228022 [details] [diff] [review]
patch as checked in (binding changes only)
Asking for 181approval now that the fix for bug 314350 is on the branch (my request here is on the entire set of patches for this bug).
Attachment #228022 -
Flags: approval1.8.1?
Comment 22•19 years ago
|
||
Comment on attachment 228022 [details] [diff] [review]
patch as checked in (binding changes only)
All patches approved for 1.8.1 drivers.
Attachment #228022 -
Flags: approval1.8.1? → approval1.8.1+
Assignee | ||
Comment 23•19 years ago
|
||
Fixed on 1.8 branch for beta 1 (branch patch is on bug 318168).
Keywords: fixed1.8.1
Whiteboard: 181b1+ branch landing is pending approval for 314350 and 343097 → 181b1+
You need to log in
before you can comment on or make changes to this bug.
Description
•