This repository has been archived by the owner on Sep 6, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Implement selection triangle in project panel #702
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
9b00223
New sidebarList util to add selection highlight and triangle indicato…
jasonsanjose 65c9864
Try positioning selection triangle to the outer file-section div
jasonsanjose e5819ac
Try clipping
jasonsanjose 8addb5a
Got fixed position and clipping working!
jasonsanjose 0f119e7
Merge branch 'refs/heads/master' into jason-sanjose/northstar-fixes-p…
jasonsanjose 186b517
Code review comments. Also added scrolling to the initial selection.
jasonsanjose 76e0602
Merge branch 'refs/heads/master' into jason-sanjose/northstar-fixes-p…
jasonsanjose ba3c5f8
Update scrollTop for every selection change. Address comments.
jasonsanjose 6c154f7
Fix scroller height and reveal edge cases.
jasonsanjose File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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; | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.clientHeight, | ||
selectionMarkerHeight = $selectionMarker.height(), | ||
selectionMarkerBottom = selectionMarkerTop + selectionMarkerHeight, | ||
currentScrollBottom = scrollerElement.scrollTop + scrollerHeight; | ||
|
||
// update scrollTop to reveal the selected list item | ||
if (selectionMarkerTop >= currentScrollBottom) { | ||
scrollerElement.scrollTop = Math.max(0, selectionMarkerTop + selectionMarkerHeight - scrollerHeight); | ||
} else if (selectionMarkerBottom <= scrollerElement.scrollTop) { | ||
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; | ||
}); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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