Highlighted node should center in the visualization (Tilt)

RESOLVED FIXED in Firefox 13

Status

()

Firefox
Developer Tools: Inspector
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: vporof, Assigned: vporof)

Tracking

12 Branch
Firefox 13
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [tilt])

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

6 years ago
Sometimes we're selecting a node using the breadcrumbs or the HTML panel which is not visible in the visualization viewport. It would be nice if the camera automatically translated/zoomed/centered/danced so we can see it.
(Assignee)

Updated

6 years ago
Assignee: nobody → vporof
Status: NEW → ASSIGNED
Whiteboard: [tilt]
(Assignee)

Updated

6 years ago
Summary: Highlighted node should center in the visualization → Highlighted node should center in the visualization (Tilt)
(Assignee)

Comment 1

6 years ago
Created attachment 592485 [details] [diff] [review]
v0

WIP.
(Assignee)

Comment 2

6 years ago
Created attachment 592500 [details] [diff] [review]
v1
Attachment #592485 - Attachment is obsolete: true
(Assignee)

Comment 3

6 years ago
Created attachment 592517 [details] [diff] [review]
v2

Fixed a few small problems with moveIntoView().
(Assignee)

Updated

6 years ago
Attachment #592500 - Attachment is obsolete: true
(Assignee)

Comment 4

6 years ago
Comment on attachment 592517 [details] [diff] [review]
v2

Green is the color: https://tbpl.mozilla.org/?tree=Try&rev=ce9d3ecd89e5
Attachment #592517 - Flags: review?
(Assignee)

Updated

6 years ago
Attachment #592517 - Flags: review? → review?(rcampbell)
(Assignee)

Comment 5

6 years ago
Created attachment 593880 [details] [diff] [review]
v2.1

Made sure bringing the node into view only happens when the selection is changed via the highlighter or breadcrumbs, and not by picking inside Tilt. I think this way is better. Rob, what do you think?
Attachment #593880 - Flags: review?(rcampbell)
(Assignee)

Updated

6 years ago
Depends on: 722129
(Assignee)

Updated

6 years ago
Blocks: 719042
(Assignee)

Updated

6 years ago
Attachment #592517 - Flags: review?(rcampbell)
Comment on attachment 593880 [details] [diff] [review]
v2.1

    *                 the index of the node in the this.traverseData array
+   * @param {String} aFlags
+   *                 flags specifying highlighting options
    */
-  highlightNodeFor: function TVP_highlightNodeFor(aNodeIndex)
+  highlightNodeFor: function TVP_highlightNodeFor(aNodeIndex, aFlags)
   {
     this.redraw = true;

do you see adding other flags in the future? Could the be better served by a flags object or even a moveIntoView boolean parameter?
(Assignee)

Comment 7

6 years ago
I think other flags are pretty feasible someday ("bounce"? "makeItPop"?). Maybe using a string for this is too hacky (the nice thing about it that instead of simply a string you could also use an array like ["moveIntoView, "makeItPop"] because indexOf).

An object may be more obvious. But then we're dealing with redundancy (like highlight(aNode, { moveIntoView: false });), since I don't see a scenario when flags should be turned off, since they're all off by default. Which approach do you prefer?
Comment on attachment 593880 [details] [diff] [review]
v2.1

I don't feel strongly about it. Providing room for future flags is an ok reason to do this and the string check isn't horrible.
Attachment #593880 - Flags: review?(rcampbell) → review+
Whiteboard: [tilt] → [tilt][land-in-fx-team]
https://hg.mozilla.org/integration/fx-team/rev/0f5003f35498
Whiteboard: [tilt][land-in-fx-team] → [tilt][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/0f5003f35498
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [tilt][fixed-in-fx-team] → [tilt]
Target Milestone: --- → Firefox 13
You need to log in before you can comment on or make changes to this bug.