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

Add/expose isVisibile, show, and hide methods for SidebarView object #3297

Merged
redmunds merged 2 commits into from
May 15, 2013
Merged

Add/expose isVisibile, show, and hide methods for SidebarView object #3297

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d5e970e
Init commit
lkcampbell Mar 30, 2013
0fee395
Small code cleanup
lkcampbell Mar 30, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 28 additions & 3 deletions src/project/SidebarView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,32 @@ define(function (require, exports, module) {
/**
* Toggle sidebar visibility.
*/
function toggleSidebar(width) {
function toggle() {
Resizer.toggle($sidebar);
}

/**
* Show the sidebar.
*/
function show() {
Resizer.show($sidebar);
}

/**
* Hide the sidebar.
*/
function hide() {
Resizer.hide($sidebar);
}

/**
* Returns the visibility state of the sidebar.
* @return {boolean} true if element is visible, false if it is not visible
*/
function visible() {
Copy link
Copy Markdown
Contributor

@TomMalbran TomMalbran May 16, 2013

Choose a reason for hiding this comment

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

I know this was merged already, but naming this function isVisible() would have been better, since it checks for the visible state and returns a boolean.

The same would apply to the new method in Resizer.js.

Maybe it could be changed as code cleanup before the Sprint ends.

Copy link
Copy Markdown
Contributor Author

@lkcampbell lkcampbell May 16, 2013

Choose a reason for hiding this comment

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

I don't have a strong opinion either way. They both sound good to me.

If you guys want me to change the name I will.

Copy link
Copy Markdown
Contributor

@redmunds redmunds May 16, 2013

Choose a reason for hiding this comment

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

Good catch, @TomMalbran. @lkcampbell Yes, please change it to isVisible().

return Resizer.visible($sidebar);
}

// Initialize items dependent on HTML DOM
AppInit.htmlReady(function () {
$sidebar = $("#sidebar");
Expand Down Expand Up @@ -128,8 +150,11 @@ define(function (require, exports, module) {
});

$(ProjectManager).on("projectOpen", _updateProjectTitle);
CommandManager.register(Strings.CMD_HIDE_SIDEBAR, Commands.VIEW_HIDE_SIDEBAR, toggleSidebar);
CommandManager.register(Strings.CMD_HIDE_SIDEBAR, Commands.VIEW_HIDE_SIDEBAR, toggle);

// Define public API
exports.toggleSidebar = toggleSidebar;
exports.toggle = toggle;
exports.show = show;
exports.hide = hide;
exports.visible = visible;
});
18 changes: 14 additions & 4 deletions src/utils/Resizer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,15 @@ define(function (require, exports, module) {
}
}

/**
* Returns the visibility state of a resizable element.
* @param {DOMNode} element Html element to toggle
* @return {boolean} true if element is visible, false if it is not visible
*/
function visible(element) {
return $(element).is(":visible");
}

/**
* Adds resizing capabilities to a given html element.
*
Expand Down Expand Up @@ -427,10 +436,11 @@ define(function (require, exports, module) {
});
});

exports.makeResizable = makeResizable;
exports.toggle = toggle;
exports.show = show;
exports.hide = hide;
exports.makeResizable = makeResizable;
exports.toggle = toggle;
exports.show = show;
exports.hide = hide;
exports.visible = visible;

//Resizer Constants
exports.DIRECTION_VERTICAL = DIRECTION_VERTICAL;
Expand Down
⚡