If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[RFE] expanding should scroll new items into view

NEW
Unassigned

Status

()

Core
XUL
P3
enhancement
16 years ago
2 years ago

People

(Reporter: neil@parkwaycc.co.uk, Unassigned)

Tracking

({polish})

Trunk
Future
x86
All
polish
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

16 years ago
<method name="_toggleOpenState">
        <parameter name="row"/>
        <body>
          <![CDATA[
            var box = this.outlinerBoxObject;
            var c = box.view.rowCount;
            box.view.toggleOpenState(row);
            var l = box.view.rowCount - c;
            if (l > 0) {
              var p = box.getPageCount();
              if (l >= p)
                box.ensureRowIsVisible(row + p - 1);
              else
                box.ensureRowIsVisible(row + l);
            }
          ]]>
        </body>
      </method>
(Reporter)

Comment 1

16 years ago
Created attachment 69241 [details] [diff] [review]
Proposed patch

Sorry for the delay :-(
(Reporter)

Updated

16 years ago
Keywords: patch, polish, review
(Reporter)

Comment 2

16 years ago
Created attachment 69247 [details] [diff] [review]
More readable
Attachment #69241 - Attachment is obsolete: true

Comment 3

16 years ago
Neil, this patch is really more readble and now I understand it better.
But let me think about it a bit.

Comment 4

15 years ago
reassigning
Assignee: hewitt → varga
Severity: minor → enhancement
OS: Windows 95 → All
Priority: -- → P3
Summary: expanding should scroll new items into view → [RFE] expanding should scroll new items into view
Target Milestone: --- → Future

Comment 5

15 years ago
Any progress on that in the meantime?

Comment 6

13 years ago
*** Bug 255618 has been marked as a duplicate of this bug. ***

Comment 7

13 years ago
Is there something wrong with this patch? Why has it not been checked in, nor
commented on in 2 and a half years?

Comment 8

11 years ago
Created attachment 266618 [details] [diff] [review]
update patch
Assignee: Jan.Varga → enndeakin
Attachment #69247 - Attachment is obsolete: true
Status: NEW → ASSIGNED

Updated

11 years ago
Duplicate of this bug: 333409
(Reporter)

Comment 10

11 years ago
Created attachment 266620 [details] [diff] [review]
handles more edge cases

My original patch assumed that the row to be toggled was already visible.
This one does not. Instead it follows Windows behaviour. Of course you may not want to follow windows behaviour, but here's a description:
* If you're collapsing, then always ensure the row is visible
* If the row to expand is off the top, then do nothing else
* If all the rows will fit, then make them fit
* Otherwise scroll the row to the top

Comment 11

10 years ago
Neil, did you want someone to review this?
(Reporter)

Comment 12

10 years ago
(In reply to comment #11)
>Neil, did you want someone to review this?
Depends. I slavishly copied what appears to be Windows behaviour, but you may want to go with something conceptually simpler, e.g. a) toggled row should always be made visible b) as many new rows as possible should be made visible.

Updated

9 years ago
Component: XP Toolkit/Widgets: Trees → XUL
QA Contact: jrgmorrison → xptoolkit.widgets

Comment 13

9 years ago
The description in comment 10 suggests that the row the user just toggled can move to a new position visibly. That seems odd to me, thinking about it. Nothing special seems to happen on Mac (at least in Finder and Safari) when toggling treeitems, so maybe this should be Windows only.

Comment 14

9 years ago
Although usually I am an opponent of trying to predict what the user wants, this is an edge case. I can't think of a time when you'd be expanding a tree node and NOT want to see the children. As it is now, if your parent node is the bottom line in a window, expanding the node appears to do nothing because the children appear outside of the window. Your next move will have to be to scroll them into view, so there's no reason I can think of not to do it automatically.

Updated

2 years ago
Assignee: enndeakin → nobody
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.