Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Implement selection triangle in project panel #702

Merged
merged 9 commits into from
Apr 21, 2012
2 changes: 2 additions & 0 deletions src/editor/CSSInlineEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ define(function (require, exports, module) {
this.$relatedContainer = $(document.createElement("div")).addClass("relatedContainer");
this._relatedContainerInserted = false;
this._relatedContainerInsertedHandler = this._relatedContainerInsertedHandler.bind(this);

// FIXME (jasonsj): deprecated event http://www.w3.org/TR/DOM-Level-3-Events/
this.$relatedContainer.on("DOMNodeInserted", this._relatedContainerInsertedHandler);

// List "selection" highlight
Expand Down
14 changes: 14 additions & 0 deletions src/project/ProjectManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ define(function (require, exports, module) {
*/
var _projectTree = null;

/**
* @private
* Reference to the tree control UL element
* @type {DOMElement}
*/
var $projectTreeList;

/**
* @private
* @see getProjectRoot()
Expand Down Expand Up @@ -79,6 +86,9 @@ define(function (require, exports, module) {
} else if (_projectTree !== null) {
_projectTree.jstree("deselect_all");
}

// redraw selection
$projectTreeList.trigger("selectionChanged");
};

$(FileViewController).on("documentSelectionFocusChange", _documentSelectionFocusChange);
Expand Down Expand Up @@ -259,6 +269,10 @@ define(function (require, exports, module) {
FileViewController.addToWorkingSetAndSelect(entry.fullPath);
}
});

// fire selection changed events for sidebarSelection
$projectTreeList = $projectTreeContainer.find("ul");
ViewUtils.sidebarList($projectTreeContainer, "jstree-clicked");
});

return result;
Expand Down
7 changes: 6 additions & 1 deletion src/project/WorkingSetView.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ define(function (require, exports, module) {
* Use listItem.data(_FILE_KEY) to get the document reference
*/
var _FILE_KEY = "file",
$openFilesContainer = $("#open-files-container");
$openFilesContainer = $("#open-files-container"),
$openFilesList = $openFilesContainer.find("ul");

function _hideShowOpenFileHeader() {
if (DocumentManager.getWorkingSet().length === 0) {
Expand Down Expand Up @@ -248,10 +249,14 @@ define(function (require, exports, module) {

$(FileViewController).on("documentSelectionFocusChange", function (event, eventTarget) {
_handleDocumentSelectionChange();

// redraw selection
$openFilesList.trigger("selectionChanged");
});

_hideShowOpenFileHeader();

// Show scroller shadows when open-files-container scrolls
ViewUtils.installScrollShadow($openFilesContainer[0]);
ViewUtils.sidebarList($openFilesContainer);
});
40 changes: 28 additions & 12 deletions src/styles/brackets.less
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ a, img {
.vbox;
.box-flex(1);
margin: 5px 0px 0px 0px;
position: relative;

.project-file-header-area {
padding: 6px 6px 2px 10px;
Expand All @@ -195,15 +196,12 @@ a, img {
border-bottom: 1px solid #9a9a9a;
box-shadow: 0px 1px #eeeeee;
}


}

#open-files-container {
.box-flex(0);
margin: 0 0 22px 0;
padding: 0px;
overflow: auto;
max-height: 200px; // TODO (Issue #276): it would be nicer to have this be 50%, but that doesn't seem to work


Expand All @@ -228,6 +226,7 @@ a, img {
color: #BBB;
font-size: 14px;
margin-left: 18px;
padding-right: @triangle-size * 2;
cursor: default;
}
.extension a {
Expand All @@ -240,8 +239,6 @@ a, img {
background-position: 0 1px;
}
&.selected {
height: 25px;
background: url("styles/images/active_back.png") no-repeat top right;
a {
font-weight: @font-weight-semibold;
color: #fff;
Expand All @@ -251,19 +248,39 @@ a, img {
}
}

.sidebarSelection {
background: url("styles/images/active_back.png") no-repeat top right;
height: 25px;
position: absolute;
}

.sidebarSelectionTriangle {
position: fixed;

margin-top: -@triangle-size;

border-top: @triangle-size solid transparent;
border-bottom: @triangle-size solid transparent;
border-right: @triangle-size solid @background-color-3;

.scaleX(0.9, right, top);
}

//Initially start with the open files hidden, they will get show as files are added
#open-files-container {
display:none;
}

#project-files-container {
.box-flex(1);
padding: 0px;
padding-left: 8px;
margin: 0;
overflow: auto;

ul { margin-top: 0; }
.jstree-brackets li > a {
padding-right: @triangle-size * 2;
}

ul {
padding-left: 8px;
}
}

.scrollerShadow {
Expand Down Expand Up @@ -418,7 +435,6 @@ a, img {
* (b) Use transform scaleX and origin to adjust width.
*/
.selection:before {
@triangle-size: 10px;
content: " ";
position: absolute;
width: 0;
Expand All @@ -428,7 +444,7 @@ a, img {
border-left: @triangle-size solid @inline-background-color-1;
margin-top: -@triangle-size;
top: 50%;
.scaleX(0.9);
.scaleX(0.9, left, top);
}

.related {
Expand Down
12 changes: 6 additions & 6 deletions src/styles/brackets_mixins.less
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,16 @@


/* Scale x-axis using top-left as the origin */
.scaleX (@value: 1.0) {
.scaleX (@value: 1.0, @horizontal: 50%, @vertical: 50%) {
-ms-transform: scaleX(@value);
-moz-transform: scaleX(@value);
-webkit-transform: scaleX(@value);
-o-transform: scaleX(@value);
transform: scaleX(@value);

-ms-transform-origin: 0 0;
-moz-transform-origin: 0 0;
-webkit-transform-origin: 0 0;
-o-transform-origin: 0 0;
transform-origin: 0 0;
-ms-transform-origin: @horizontal @vertical;
-moz-transform-origin: @horizontal @vertical;
-webkit-transform-origin: @horizontal @vertical;
-o-transform-origin: @horizontal @vertical;
transform-origin: @horizontal @vertical;
}
1 change: 0 additions & 1 deletion src/styles/brackets_patterns_override.less
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@

padding: 0;
margin: 0;
border-right: 1px solid #4e5153;

.project-panel-background;
color: #bbb;
Expand Down
5 changes: 4 additions & 1 deletion src/styles/brackets_variables.less
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,7 @@
/* Common font sizes */
@title-font-size: 18px; // headings such as the editor titlebar
@label-font-size: 14px; // labels on buttons, menubar, etc.
@menu-item-font-size: 13px; // individual menu items
@menu-item-font-size: 13px; // individual menu items

/* CSS triangle */
@triangle-size: 10px;
1 change: 0 additions & 1 deletion src/styles/jsTreeTheme.less
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
&.jstree-clicked {
color: #fff;
font-weight: @font-weight-semibold;
background: url("styles/images/active_back.png") no-repeat top right;
}
}
.extension a {
Expand Down
109 changes: 108 additions & 1 deletion src/utils/ViewUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Copy link
Contributor

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.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

$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", "");
}
Copy link
Contributor

Choose a reason for hiding this comment

The 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.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, nice catch. I'll take a look.

Copy link
Member Author

Choose a reason for hiding this comment

The 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(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be scrollerElement.clientHeight instead, so it doesn't include the height of the bottom scrollbar. (With the current code, the item gets partially obscured by the scrollbar.)

Copy link
Member Author

Choose a reason for hiding this comment

The 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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be >=, I think (right now I'm hitting an edge case where if the bottommost item in the working set is selected and you open another file from the file tree, the working set doesn't scroll to show it because its top is right at the bottom of the scroller).

Copy link
Member Author

Choose a reason for hiding this comment

The 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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might need to be <= too, although I didn't test this case.

Copy link
Member Author

Choose a reason for hiding this comment

The 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;
});