* SUMMARY TimelineRuler should have a "select all" mode. When active, selection handles should be hidden, and the selection start and end properties should return the ruler's start and ends times. Clicking & dragging to create a new selection should cause the ruler to no longer "select all". UI making use of this mode will be added in: https://bugs.webkit.org/show_bug.cgi?id=153033.
<rdar://problem/24779872>
Created attachment 271964 [details] [Patch] Proposed Fix
Comment on attachment 271964 [details] [Patch] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=271964&action=review > Source/WebInspectorUI/UserInterface/Views/TimelineRuler.js:265 > + set selectAll(x) entireRangeSelected? select, as a verb bothers me, as a property name. > Source/WebInspectorUI/UserInterface/Views/TimelineRuler.js:297 > + return this._selectAll ? this._endTime : this._selectionEndTime; This is still a bounded time. I suspect this will cause things to filter out when you zoom in to tighten the time range. Does this result get used?
(In reply to comment #3) > Comment on attachment 271964 [details] > [Patch] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=271964&action=review > > > Source/WebInspectorUI/UserInterface/Views/TimelineRuler.js:265 > > + set selectAll(x) > > entireRangeSelected? select, as a verb bothers me, as a property name. > > > Source/WebInspectorUI/UserInterface/Views/TimelineRuler.js:297 > > + return this._selectAll ? this._endTime : this._selectionEndTime; > > This is still a bounded time. I suspect this will cause things to filter out > when you zoom in to tighten the time range. Does this result get used? We could use the default selection bounds (0 and Infinity), and make the necessary adjustments to timeline views.
(In reply to comment #4) > (In reply to comment #3) > > Comment on attachment 271964 [details] > > [Patch] Proposed Fix > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=271964&action=review > > > > > Source/WebInspectorUI/UserInterface/Views/TimelineRuler.js:265 > > > + set selectAll(x) > > > > entireRangeSelected? select, as a verb bothers me, as a property name. > > > > > Source/WebInspectorUI/UserInterface/Views/TimelineRuler.js:297 > > > + return this._selectAll ? this._endTime : this._selectionEndTime; > > > > This is still a bounded time. I suspect this will cause things to filter out > > when you zoom in to tighten the time range. Does this result get used? > > We could use the default selection bounds (0 and Infinity), and make the > necessary adjustments to timeline views. That would be easier to understand.
(In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > > Comment on attachment 271964 [details] > > > [Patch] Proposed Fix > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=271964&action=review > > > > > > > Source/WebInspectorUI/UserInterface/Views/TimelineRuler.js:265 > > > > + set selectAll(x) > > > > > > entireRangeSelected? select, as a verb bothers me, as a property name. > > > > > > > Source/WebInspectorUI/UserInterface/Views/TimelineRuler.js:297 > > > > + return this._selectAll ? this._endTime : this._selectionEndTime; > > > > > > This is still a bounded time. I suspect this will cause things to filter out > > > when you zoom in to tighten the time range. Does this result get used? > > > > We could use the default selection bounds (0 and Infinity), and make the > > necessary adjustments to timeline views. > > That would be easier to understand. The grid-based timeline views handle this fine, but OverviewTimelineView and MemoryTimelineView will need special handling for selectionEndTime === Infinity.
Created attachment 272090 [details] [Video] "select all" behavior Doubling-clicking the ruler toggles the ruler between the user-defined selection (if any), and "everything selected". It's a convenient interaction for the initiated, but not very discoverable. Upcoming UI (see https://webkit.org/b/153033) will make it accessible.
Created attachment 272093 [details] [Patch] Proposed Fix
Comment on attachment 272093 [details] [Patch] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=272093&action=review > Source/WebInspectorUI/UserInterface/Views/TimelineRuler.js:304 > + return this._selectionStartTime === this._zeroTime && this._selectionEndTime === Number.MAX_VALUE; You might need to add this in set zeroTime(). if (this.entireRangeSelected) { this._selectionStartTime = newZeroTime; } > Source/WebInspectorUI/UserInterface/Views/TimelineRuler.js:708 > + _handleDoubleClick(event) > + { > + if (this.entireRangeSelected) { > + this.selectionStartTime = this._lastSelectionStartTime; > + this.selectionEndTime = this._lastSelectionEndTime; > + } else { > + this._lastSelectionStartTime = this.selectionStartTime; > + this._lastSelectionEndTime = this.selectionEndTime; > + > + this.selectEntireRange(); > + } > + } I like double click to get the entire range. I don't think double click to go back to the previous selection is as clear or compelling. The user can always just drag out a new selection. But this is fine as-is.
Created attachment 272115 [details] [Patch] Proposed Fix
Comment on attachment 272115 [details] [Patch] Proposed Fix Clearing flags on attachment: 272115 Committed r197034: <http://trac.webkit.org/changeset/197034>
All reviewed patches have been landed. Closing bug.
Comment on attachment 272115 [details] [Patch] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=272115&action=review > Source/WebInspectorUI/UserInterface/Views/TimelineRuler.js:179 > + if (this.entireRangeSelected) > + this.selectionStartTime = this._zeroTime; This needed to happen before "this._zeroTime = x", otherwise the getter for entireRangeSelected will fail, because _selectionStartTime will not equal _zeroTime anymore.
I'll land a follow up fix.
Landed a follow up in https://trac.webkit.org/r197035.