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

Update Electron to 1.6.10 #543

Merged
merged 1 commit into from
Jun 2, 2017
Merged

Conversation

yuya-oc
Copy link
Contributor

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

Before submitting, please confirm you've

Please provide the following information:

Summary
Update Electron to v1.6.10. This is the prior work for #525.

Electron 1.6.7 supports following features for Windows. So this PR should test them.

  • Desktop notification for Windows 7
  • Per-monior DPI awareness

Issue link
#67, #357

Test Cases
Notification:

  1. Prepare Windows 7, 8 or 8.1
  2. Receive any message.
  3. Notification should appear instead of a tray balloon.
  4. Click the notification.
  5. The message should appear in the main window as well as Windows 10.

Per-monitor DPI:

  1. Prepare Windows 8.1 or 10
  2. Connect two monitors and set different DPI scaling.
  3. Move the window across monitors.
  4. The window should be rendered in proper DPI.

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

- Desktop notification for Windows 7
- Per-monitor DPI awareness
@jasonblais
Copy link
Contributor

I tested desktop notifications on Windows 10, works as expected. I'll ask a team member to help with Windows 7.

I only have one monitor so I can't test the second improvement..I'm not sure if you're able to?

@lindy65
Copy link

lindy65 commented Jun 1, 2017

I tested desktop notifications on Windows 7.

While desktop notifications work as expected for direct messages and public channels, they do not appear for messages or at mentions sent in private channels, even though I am a member of the channel and have notifications set to "all activity".

Although the desktop notification does not fire for private channels, the channel is bolded and shows number of mentions.

image

image

@yuya-oc
Copy link
Contributor Author

yuya-oc commented Jun 1, 2017

I tested per-monitor DPI. It looks fine also on the secondary monitor.

Case 1: Primary 150%, Secondary 100%

v3.7.0 on 100%
v3.7.0-100

PR on 100%
PR-100

Case 2: Primary 100%, Secondary 150%

v3.7.0 on 150%
v3.7.0-150

PR on 150%
PR-150

@jasonblais
Copy link
Contributor

@yuya-oc Lindy helped test on the browser and the issue she reports reproduces on Chrome as well

So we should be okay to merge this PR

@razzeee
Copy link
Contributor

razzeee commented Jun 2, 2017

Would there be interest in moving this application to typescript? As Electron now bundles the definitions? https://electron.atom.io/blog/2017/06/01/typescript

@yuya-oc
Copy link
Contributor Author

yuya-oc commented Jun 2, 2017

Sounds nice, but probably there are not enough time to move for me, so currently no. I feel we should consider to improve code structure rather than that.

(And I have no experience in TypeScript)

@yuya-oc
Copy link
Contributor Author

yuya-oc commented Jun 2, 2017

@jasonblais @lindy65 Thanks for testing!

@yuya-oc yuya-oc merged commit c9d5912 into mattermost:master Jun 2, 2017
@yuya-oc yuya-oc deleted the update-electron branch June 2, 2017 14:42
@lindy65
Copy link

lindy65 commented Jun 2, 2017

Pleasure @yuya-oc! Anytime :)

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.

4 participants