Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove sub-folders from open folders array when closing it's parent #130

Merged
merged 6 commits into from
May 19, 2016
Merged

Remove sub-folders from open folders array when closing it's parent #130

merged 6 commits into from
May 19, 2016

Conversation

ortichon
Copy link
Member

@ortichon ortichon commented May 19, 2016 •
edited
Loading

Change Summary

More info

the function iterates trough all the current sub-folders, checks if they have additional open sub-folders, and remove them all from the expandedNodes array

Before you submit a PR, make sure you did the following things:

  • did you link this PR to an issue?

Make sure there's an issue open about the change you did (open one if there isn't). link this PR to that issue by writing resolves #{{issue_number}}. If this PR had several mission, link each issue in its parallel mission.

  • did you lint your changes to both javascript and scss?

hound can be pretty rough at keeping our code clean and readable! make sure you format your code by running either gulp format or gulp lint and cleaning out all the _lint errors. If you won't, you'll have a _long conversation with hound :-)

  • "I'm pretty sure I'll be able to read and understand this PR, even if I wasn't the author." - _said the PR author_

@thatkookooguy thatkookooguy temporarily deployed to kibibit-demo-pr-130 May 19, 2016 15:36 Inactive
houndci-bot
houndci-bot reviewed May 19, 2016
View reviewed changes
public/app/components/fileTree/fileTree.js
if (isFolderOpen) {
vm.expandedNodes.splice(folderIndex, 1);
}
}
Copy link

Choose a reason for hiding this comment

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

Missing semicolon after statement

@thatkookooguy thatkookooguy temporarily deployed to kibibit-demo-pr-130 May 19, 2016 15:39 Inactive
@thatkookooguy
Copy link
Member

Recursive is a bit of an overkill here. Basically, because every path of every child will have the collapsed folder as a prefix, you can just go over the array and remove all items that have the path as a prefix

@thatkookooguy
Copy link
Member

Great for making our convention forthe "folder" name more consistent

@thatkookooguy
Copy link
Member

You can use this to filter the strings you don't want
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/filter

@thatkookooguy thatkookooguy temporarily deployed to kibibit-demo-pr-130 May 19, 2016 17:29 Inactive
houndci-bot
houndci-bot reviewed May 19, 2016
View reviewed changes
public/app/components/fileTree/fileTree.js
var closeFolder = function(folder) {
var nameIndex = folder.path.indexOf(folder.name);
var pathPrefix = folder.path.substr(0, nameIndex) + folder.name;

Copy link

Choose a reason for hiding this comment

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

Illegal trailing whitespace

Copy link
Member Author

@ortichon ortichon May 19, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

@houndci-bot
good boy, now work again!

@thatkookooguy thatkookooguy temporarily deployed to kibibit-demo-pr-130 May 19, 2016 17:32 Inactive
@thatkookooguy
Copy link
Member

Sorry for the nits, but maybe use
https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/String/startsWith
Instead of indexOf to make sure this is indeed a subfolder

@ortichon
Copy link
Member Author

@thatkookooguy
Great, somewhere in my mind/heart I knew there is a function for this. :)

@thatkookooguy
Copy link
Member

@ortichon Haha same here. Didn't new about it before. Just tried to look something up 😂

@thatkookooguy thatkookooguy temporarily deployed to kibibit-demo-pr-130 May 19, 2016 17:45 Inactive
@thatkookooguy
Copy link
Member

thatkookooguy commented May 19, 2016 •
edited by neilkalman-redkix
Loading

LGTM

Approved with PullApprove

@thatkookooguy thatkookooguy merged commit 2395c3b into master May 19, 2016
@thatkookooguy thatkookooguy deleted the remove-subfolders-from-folder-array branch June 30, 2016 10:47
neilkalman-redkix pushed a commit that referenced this pull request Jul 4, 2016
@thatkookooguy
Merge pull request #130 from Kibibit/remove-subfolders-from-folder-array
9b7b0ca 
Remove sub-folders from open folders array when closing it's parent
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@thatkookooguy thatkookooguy

Labels
BUG Front End
Projects
None yet
Milestone
01. Create a kickass **code editor** ...
Development

Successfully merging this pull request may close these issues.

sub-folders aren't removed from the open folders array
3 participants
@ortichon @thatkookooguy @houndci-bot