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.

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

Forward by f #7

Merged
merged 3 commits into from
Mar 13, 2014
Merged

Forward by f #7

merged 3 commits into from
Mar 13, 2014

Conversation

roderik
Copy link
Contributor

@roderik roderik commented Mar 12, 2014

Made a bad branch, this also contains the MessageView commit. So merge this one after the previous one.

Roderik van der Veer added 2 commits March 12, 2014 17:15
…have a message in a conversation view selected or directly in a message)
@nompute
Copy link
Owner

nompute commented Mar 12, 2014

Hmm, I'm getting an invalid selector error when trying to use "f" to forward. The message view navigation isn't working on 7.2 either (I think Mail 7 broke the old functionality). Are you using 7.2 as well? I'm going to take a closer look in the evening.

@roderik
Copy link
Contributor Author

roderik commented Mar 12, 2014

I'm on 7.2 as well. Using Xcode 5.1.
Forward does work in the list to the left, but not in the message view. Reply does in the message view.

I'm on a belgian azerty macbook air btw where cmd-shift-f is forward, maybe different on a qwerty macbook?

@nompute
Copy link
Owner

nompute commented Mar 12, 2014

Hmm, Cmd+Shift+f is the "forward" shortcut, so it should be the same. Still, could be related to keyboard settings. I'll do some spelunking and get back to you.

@nompute
Copy link
Owner

nompute commented Mar 13, 2014

Ah, found the problem. In overrideMessagesKeyDown, in your case block for handling forwarding, you're calling out to overrideMailKeyDown instead. There's some other odd behavior, though, like "reply-all" suddenly becoming only "reply", but I think I have other ways around that. Why don't you fix the commit, and I'll go ahead and merge it first before making other changes.

@roderik
Copy link
Contributor Author

roderik commented Mar 13, 2014

Ok, seems to work fine now. I'm not seeing this wrong a/r behaviour

nompute added a commit that referenced this pull request Mar 13, 2014
@nompute nompute merged commit 1f04ada into nompute:master Mar 13, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants