Added button to remove projects from the recent project dropdown. #1757

Merged
merged 3 commits into from Oct 17, 2012

Projects

None yet

2 participants

@adrocknaphobia
Member

No description provided.

@redmunds redmunds was assigned Oct 3, 2012
@redmunds redmunds commented on the diff Oct 3, 2012
src/extensions/default/RecentProjects/close_btn.svg
@@ -0,0 +1,15 @@
+<svg version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="45px" height="16px">
+ <defs>
+ <filter id="blur" x="-10" y="-10" width="12" height="12">
+ <feGaussianBlur in="SourceGraphic" stdDeviation=".25"/>
+ </filter>
+ <polygon id="cross" points="12.207,4.207 10.793,2.793 7.5,6.086 4.207,2.793 2.793,4.207 6.086,7.5 2.793,10.793 4.207,12.207 7.5,8.914 10.793,12.207 12.207,10.793 8.914,7.5"/>
+ </defs>
+
+ <use xlink:href="#cross" y="1" filter="url(#blur)" opacity=".5"/>
+ <use xlink:href="#cross" fill="#868686"/>
+ <use xlink:href="#cross" x="15" y="1" filter="url(#blur)" opacity=".5"/>
+ <use xlink:href="#cross" x="15" fill="#9f9f9f"/>
+ <circle cx="37" cy="8" r="3" filter="url(#blur)" opacity=".5"/>
+ <circle fill="#868686" cx="37" cy="7" r="3"/>
+</svg>
@redmunds
redmunds Oct 3, 2012 collaborator

There is already an X icon used for the Working Set list. I think you should be able to re-use that one.

@redmunds
redmunds Oct 15, 2012 collaborator

On second thought, it's probably better to use your own icon so there's no dependency.

@redmunds redmunds commented on an outdated diff Oct 3, 2012
src/extensions/default/RecentProjects/styles.css
@@ -6,6 +6,14 @@
background-image: url("down-arrow.png");
}
+.trash-icon {
+ display: inline;
+ width: 16px;
+ height: 16px;
+ background-image: url("close_btn.svg");
+ position: absolute;
@redmunds
redmunds Oct 3, 2012 collaborator

Since you're not re-positioning icon (e.g. left/top), then I don't think you need position:absolute.

@redmunds
redmunds Oct 3, 2012 collaborator

Ah, I see that you're positioning it dynamically. Nevermind. :)

@redmunds redmunds commented on an outdated diff Oct 3, 2012
src/extensions/default/RecentProjects/styles.css
@@ -6,6 +6,14 @@
background-image: url("down-arrow.png");
}
+.trash-icon {
+ display: inline;
@redmunds
redmunds Oct 3, 2012 collaborator

You're dynamically changing display to 'inline-block', so you probably don't need to set it to 'inline' here.

@redmunds redmunds commented on an outdated diff Oct 3, 2012
src/extensions/default/RecentProjects/main.js
@@ -52,7 +52,7 @@ define(function (require, exports, module) {
var prefs = PreferencesManager.getPreferenceStorage(PREFERENCES_KEY),
recentProjects = prefs.getValue("recentProjects") || [],
i;
- for (i = 0; i < recentProjects.length; i++) {
+ for (i = 0; i < recentProjects.length; i++) {
@redmunds
redmunds Oct 3, 2012 collaborator

Need to use 4 spaces instead of Tabs. This applies to all edits.

@redmunds redmunds commented on an outdated diff Oct 3, 2012
src/extensions/default/RecentProjects/main.js
@@ -148,13 +152,45 @@ define(function (require, exports, module) {
}
});
closeDropdown();
- });
- $("<li></li>")
+ })
+ .mouseenter(function (e) {
+ var $target = $(e.currentTarget),
+ $del = renderDelete()
+ .click(function () {
@redmunds
redmunds Oct 3, 2012 collaborator

.click() function call should be indented.

@redmunds redmunds commented on an outdated diff Oct 3, 2012
src/extensions/default/RecentProjects/main.js
@@ -148,13 +152,45 @@ define(function (require, exports, module) {
}
});
closeDropdown();
- });
- $("<li></li>")
+ })
+ .mouseenter(function (e) {
+ var $target = $(e.currentTarget),
+ $del = renderDelete()
+ .click(function () {
+ // remove the project from the preferences.
+ var prefs = PreferencesManager.getPreferenceStorage(PREFERENCES_KEY),
+ recentProjects = getRecentProjects(),
+ index = recentProjects.indexOf($(this).data("path")),
+ newProjects = [],
+ i;
+ for (i = 0; i < recentProjects.length; i++) {
+ if(i !== index) {
@redmunds
redmunds Oct 3, 2012 collaborator

need a space between "if" and "(". I think JSLint would catch that, so be sure JSLint is turned on.

@redmunds
Collaborator
redmunds commented Oct 3, 2012

Done with initial review. Very cool, but needs some code cleanup work.

Adam Lehman added some commits Oct 17, 2012
@adrocknaphobia
Member

Well... I cleaned up the code, but not sure I pushed it correctly. Am I supposed to have a second commit (e0f2674)?

@redmunds
Collaborator

@adrocknaphobia The second commit is ok. It is due to you merging in all of the changes to master which have occurred since you first submitted this pull request. If you switch to the "Files Changed" tab, you can see that only your changes are listed.

Looks good. Merging.

@redmunds redmunds merged commit 772fec1 into master Oct 17, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment