-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Implement selection triangle in project panel #702
Changes from 8 commits
9b00223
65c9864
e5819ac
8addb5a
0f119e7
186b517
76e0602
ba3c5f8
6c154f7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,7 +26,6 @@ | |
|
||
padding: 0; | ||
margin: 0; | ||
border-right: 1px solid #4e5153; | ||
|
||
.project-panel-background; | ||
color: #bbb; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,9 +58,116 @@ define(function (require, exports, module) { | |
// update immediately | ||
_updateScrollerShadow(displayElement, scrollElement); | ||
} | ||
|
||
/** | ||
* Within a scrolling DOMElement, creates and positions a styled selection | ||
* div to align a single selected list item from a ul list element. | ||
* | ||
* Assumptions: | ||
* - scrollElement is a child of the #file-section div | ||
* - ul list element fires a "selectionChanged" event after the | ||
* selectedClassName is assigned to a new list item | ||
* | ||
* @param {!DOMElement} scrollElement A DOMElement containing a ul list element | ||
* @param {!string} selectedClassName A CSS class name on at most one list item in the contained list | ||
*/ | ||
function sidebarList($scrollerElement, selectedClassName) { | ||
var $listElement = $scrollerElement.find("ul"), | ||
$selectionMarker, | ||
$selectionTriangle, | ||
$fileSection = $("#file-section"); | ||
|
||
// build selectionMarker and position absolute within the scroller | ||
$selectionMarker = $(document.createElement("div")).addClass("sidebarSelection"); | ||
$scrollerElement.prepend($selectionMarker); | ||
|
||
// enable scrolling | ||
$scrollerElement.css("overflow", "auto"); | ||
|
||
// use relative postioning for clipping the selectionMarker within the scrollElement | ||
$scrollerElement.css("position", "relative"); | ||
|
||
// build selectionTriangle and position fixed to the window | ||
$selectionTriangle = $(document.createElement("div")).addClass("sidebarSelectionTriangle"); | ||
$fileSection.append($selectionTriangle); | ||
|
||
selectedClassName = "." + (selectedClassName || "selected"); | ||
|
||
var updateSelectionTriangle = function () { | ||
var scrollerOffset = $scrollerElement.offset(), | ||
scrollerTop = scrollerOffset.top, | ||
scrollerBottom = scrollerTop + $scrollerElement.get(0).clientHeight, | ||
scrollerLeft = scrollerOffset.left, | ||
triangleTop = $selectionMarker.offset().top, | ||
triangleHeight = $selectionTriangle.outerHeight(), | ||
triangleOffsetYBy = $selectionMarker.height() / 2, | ||
triangleClipOffsetYBy = Math.floor(($selectionMarker.height() - triangleHeight) / 2), | ||
triangleBottom = triangleTop + triangleHeight + triangleClipOffsetYBy; | ||
|
||
$selectionTriangle.css("top", triangleTop + triangleOffsetYBy); | ||
$selectionTriangle.css("left", $fileSection.width() - $selectionTriangle.outerWidth()); | ||
|
||
if (triangleTop < scrollerTop || triangleBottom > scrollerBottom) { | ||
$selectionTriangle.css("clip", "rect(" + Math.max(scrollerTop - triangleTop - triangleClipOffsetYBy, 0) + "px, auto, " + | ||
(triangleHeight - Math.max(triangleBottom - scrollerBottom, 0)) + "px, auto)"); | ||
} else { | ||
$selectionTriangle.css("clip", ""); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the list container resizes taller, do we need to detect that and update the triangle's clip region? (I think we listen for window resize in the rule list, which happens to handle this case, although I think we actually installed it for a different reason there.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, nice catch. I'll take a look. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
}; | ||
|
||
var updateSelectionMarker = function () { | ||
// find the selected list item | ||
var $listItem = $listElement.find(selectedClassName).closest("li"); | ||
|
||
if ($listItem.length === 1) { | ||
// list item position is relative to scroller | ||
var selectionMarkerTop = $listItem.offset().top - $scrollerElement.offset().top + $scrollerElement.get(0).scrollTop; | ||
|
||
// force selection width to match scroller | ||
$selectionMarker.width($scrollerElement.get(0).scrollWidth); | ||
|
||
// move the selectionMarker position to align with the list item | ||
$selectionMarker.css("top", selectionMarkerTop); | ||
$selectionMarker.show(); | ||
|
||
updateSelectionTriangle(); | ||
|
||
$selectionTriangle.show(); | ||
|
||
// fully scroll to the selectionMarker if it's not initially in the viewport | ||
var scrollerElement = $scrollerElement.get(0), | ||
scrollerHeight = $scrollerElement.height(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
selectionMarkerHeight = $selectionMarker.height(), | ||
selectionMarkerBottom = selectionMarkerTop + selectionMarkerHeight, | ||
currentScrollBottom = scrollerElement.scrollTop + scrollerHeight; | ||
|
||
// update scrollTop to reveal the selected list item | ||
if (selectionMarkerTop > currentScrollBottom) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
scrollerElement.scrollTop = Math.max(0, selectionMarkerTop + selectionMarkerHeight - scrollerHeight); | ||
} else if (selectionMarkerBottom < scrollerElement.scrollTop) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might need to be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
scrollerElement.scrollTop = selectionMarkerTop; | ||
} | ||
} else { | ||
// hide the selection marker when no selection is found | ||
$selectionTriangle.hide(); | ||
$selectionMarker.hide(); | ||
} | ||
}; | ||
|
||
$listElement.on("selectionChanged", updateSelectionMarker); | ||
$scrollerElement.on("scroll", updateSelectionTriangle); | ||
|
||
// update immediately | ||
updateSelectionMarker(); | ||
|
||
// update clipping when the window resizes | ||
$(window).on("resize", updateSelectionTriangle); | ||
} | ||
|
||
// Define public API | ||
exports.SCROLL_SHADOW_HEIGHT = SCROLL_SHADOW_HEIGHT; | ||
|
||
exports.updateChildrenToParentScrollwidth = updateChildrenToParentScrollwidth; | ||
exports.installScrollShadow = installScrollShadow; | ||
exports.SCROLL_SHADOW_HEIGHT = SCROLL_SHADOW_HEIGHT; | ||
exports.sidebarList = sidebarList; | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the vars associated with a particular closure (e.g. scrollerOffset) should be declared in that closure, unless we're really using those vars in both closures. (I don't think this violates JSLint or js's var hoisting--vars get hoisted to the top of the closure they're declared in, not the top of the outermost function.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed