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

[#10228] Cmd+Delete now triggers Don't Save in Save Changes dialog #10459

Merged
merged 1 commit into from
Mar 3, 2015
Merged

[#10228] Cmd+Delete now triggers Don't Save in Save Changes dialog #10459

merged 1 commit into from
Mar 3, 2015

Conversation

lucaslouca
Copy link
Contributor

Cmd+Delete (Mac) now triggers Don't Save in Save Changes dialog. This is the system's default behaviour (see http://support.apple.com/en-us/HT201236).

(Issue #10228)

@@ -180,7 +180,7 @@ define(function (require, exports, module) {
}
} else if (brackets.platform === "mac") {
// CMD+D Don't Save
Copy link
Contributor

Choose a reason for hiding this comment

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

You should rather call this CMD+Delete, right?
Also, I think CMD+D still should trigger Don't Save, shouldn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello Marcel,

according to Apple CMD-Backspace is the default for Don't Save. Or did I got it wrong :/

Anyhow I have updated the comment. Thanks for the tip.

Regards,
Lucas L.

On 01 Feb 2015, at 14:29, Marcel Gerber <notifications@github.com mailto:notifications@github.com> wrote:

In src/widgets/Dialogs.js #10459 (comment):

@@ -180,7 +180,7 @@ define(function (require, exports, module) {
}
} else if (brackets.platform === "mac") {
// CMD+D Don't Save
You should rather call this CMD+Delete, right?
Also, I think CMD+D still should trigger Don't Save, shouldn't it?


Reply to this email directly or view it on GitHub https://github.com/adobe/brackets/pull/10459/files#r23897530.

Copy link
Contributor

Choose a reason for hiding this comment

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

From what I can tell (not that I have a Mac...), it's Cmd+Delete:
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello,

Apple keyboards sometimes have only one key for deletions (that is, one key that acts as a backspace and delete key). This key is located on the upper right corner of the keyboard. For instance on my MacBook Pro its labeled 'Delete' and on my wireless Apple keyboard it is just labeled with a left pointing arrow ('<----'). So hitting CMD+Backspace/Delete triggers the Don't Save action.

CMD+D is also bind on the "Duplicate" action in Brackets and doesn't seem to trigger Don't Save at all under the Mac.

I have tested the Don't Save behaviour on Apple's standard apps (like TextEdit, Preview) and CMD+D doesn't seem to trigger Don't Save either.
Whereas CMD+Backspace does.

Best Regards,
Lucas

On 01 Feb 2015, at 15:13, Marcel Gerber notifications@github.com wrote:

In src/widgets/Dialogs.js #10459 (comment):

@@ -180,7 +180,7 @@ define(function (require, exports, module) {
}
} else if (brackets.platform === "mac") {
// CMD+D Don't Save
From what I can tell (not that I have a Mac...), it's Cmd+Delete:
https://cloud.githubusercontent.com/assets/2641501/5992320/ba5a6c92-aa24-11e4-8bd8-00da4d6168f9.png

Reply to this email directly or view it on GitHub https://github.com/adobe/brackets/pull/10459/files#r23897756.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah ok, didn't know that. As stated above, I don't own a Macbook.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, at least with the keyboards that I own from Apple that is the case. If I'm not mistaken, on older versions of Mac OS X
the Don't Save action was bound to CMD+D... but not anymore :\

On 01 Feb 2015, at 15:57, Marcel Gerber <notifications@github.com mailto:notifications@github.com> wrote:

In src/widgets/Dialogs.js #10459 (comment):

@@ -180,7 +180,7 @@ define(function (require, exports, module) {
}
} else if (brackets.platform === "mac") {
// CMD+D Don't Save
Ah ok, didn't know that. As stated above, I don't own a Macbook.


Reply to this email directly or view it on GitHub https://github.com/adobe/brackets/pull/10459/files#r23898065.

@peterflynn
Copy link
Member

@lucaslouca It sounds like some Mac apps use Cmd-D also (e.g. #283 mentions this), so this should probably be ||ed with the original check.

Also, could you squash this down to one commit? 3-4 commits for a single-line change makes the git history a little sloppy.

Looks great otherwise though!

@peterflynn
Copy link
Member

Ah, I forgot that the existing Cmd-D code has been broken for a long time, due to conflicting with a menubar shortcut (see #4135 / scoped keybindings story). So this actually looks fine as is -- if you can just squash the commits, I think we can go ahead & merge.

Fix memory leak in CSS & SVG Code Hints

Clicking an Extension Manager tab while still loading will show it

Use function to clear search

Code review changes

Change font weight of menu items to be 'normal' instead of 200, to improve readability.

Changed comment to apply for CMD+Backspace

[#10228] Cmd+Delete now triggers Don't Save in Save Changes dialog
@lucaslouca
Copy link
Contributor Author

Hello,

done :)

On 2015-03-01 09:04, Peter Flynn wrote:

Ah, I forgot that the existing Cmd-D code has been broken for a long
time, due to conflicting with a menubar shortcut (see #4135 [1] /
scoped keybindings story [2]). So this actually looks fine as is -- if
you can just squash the commits, I think we can go ahead & merge.

Reply to this email directly or view it on GitHub [3].

Links:

[1] #4135
[2]
https://trello.com/c/I07x1pO0/1002-more-flexible-context-sensitive-scoped-key-event-handling
[3] #10459 (comment)

@peterflynn
Copy link
Member

Looks good -- thanks!

peterflynn added a commit that referenced this pull request Mar 3, 2015
[#10228] Cmd+Backspace now triggers "Don't Save" dialog button
@peterflynn peterflynn merged commit 55bd215 into adobe:master Mar 3, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants