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

Use sheet-end event to handle focus when closing macOS file upload dialog #525

Merged
merged 1 commit into from
Jun 6, 2017

Conversation

yuya-oc
Copy link
Contributor

@yuya-oc yuya-oc commented May 2, 2017

Before submitting, please confirm you've

Please provide the following information:

Summary
Keep focus when closing file upload dialog on macOS.

This requires Electron 1.6.8 which is still beta.

Issue link
#341

Test Cases
On Mac:

  1. Upload file using CMD or CTRL + U
  2. Choose file with keyboard + hit enter to select
  3. Hit enter to send the message

Additional Notes
https://circleci.com/gh/yuya-oc/desktop/240#artifacts

@jasonblais
Copy link
Contributor

Sorry @yuya-oc I forgot about this one. I'll ask someone on our team to help test it.

@lindalumitchell
Copy link

@jasonblais @yuya-oc Looks great! I tested on OSX on the test build here: https://240-67276967-gh.circle-artifacts.com/0/tmp/circle-artifacts.dj6jLhv/mattermost-desktop-3.7.0-mac.tar.gz and in all cases focus stayed in the text box during and after attachment upload.

Tested using CMD-U, copy/paste, drag-and-drop, and paper clip icon. Tested center and RHS, single and multiple attachments, with and without text in the post, desktop view and mobile view. Tested clicking to submit file choice and pressing Enter to submit. Tested viewing file preview and downloading attachments. All looks good; no issues found.

package.json Outdated
@@ -46,7 +46,7 @@
"chai": "^3.5.0",
"cross-env": "^3.1.4",
"devtron": "^1.4.0",
"electron": "1.6.2",
"electron": "1.6.8",
Copy link
Contributor

Choose a reason for hiding this comment

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

1.6.8 is no longer in Beta so it might be okay to merge this.

@yuya-oc yuya-oc mentioned this pull request May 31, 2017
3 tasks
@yuya-oc
Copy link
Contributor Author

yuya-oc commented Jun 2, 2017

@jasonblais Rebased due to conflicting of package.json. The changes were already tested. So I think we can merge this after simple testing.

https://circleci.com/gh/yuya-oc/desktop/269#artifacts

@yuya-oc yuya-oc removed the Pending label Jun 2, 2017
@jasonblais
Copy link
Contributor

Sounds good, I'll ping Linda next week. Great to have this fixed :)

@yuya-oc
Copy link
Contributor Author

yuya-oc commented Jun 6, 2017

Tested by Linda. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants