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

Set duration for Windows 7 notifications based on Control Panel #10517

Merged
merged 1 commit into from Sep 25, 2017

Conversation

Projects
None yet
3 participants
@yuya-oc
Contributor

yuya-oc commented Sep 14, 2017

[Control Panel] -> [Ease of Access] -> [Make it easier to focus on tasks] -> [Adjust time limits and flashing visuals] can set the duration to show a balloon. In this PR, desktop notifications would follow the setting as well. So users can set the desired duration (default value is 5 seconds).

@alespergl

This comment has been minimized.

Contributor

alespergl commented Sep 21, 2017

Thank you very much for this PR! I love it.
I would just ask if you could please change the raw registry access to an API call as per this document:
https://msdn.microsoft.com/en-us/library/windows/desktop/dd373591
🙇

@alespergl

Please change raw registry access to a call to SystemParametersInfo as described here:
https://msdn.microsoft.com/en-us/library/windows/desktop/dd373591

@yuya-oc

This comment has been minimized.

Contributor

yuya-oc commented Sep 22, 2017

Updated. Thank you for your suggestion.

@miniak

Please remove the tchar.h header include

DWORD duration;
DWORD size = sizeof(DWORD);
auto result = RegGetValue(HKEY_CURRENT_USER,
_T("Control Panel\\Accessibility"),

This comment has been minimized.

@miniak

miniak Sep 22, 2017

Contributor

Don't use tchar, use wchar_t instead. Electron is only ever built as Unicode app.

This comment has been minimized.

@miniak

miniak Sep 22, 2017

Contributor

I also suggest moving the code into a separate helper function with appropriate name

This comment has been minimized.

@miniak

miniak Sep 22, 2017

Contributor

I would also try using base/win/registry.h

@miniak

SystemParametersInfo returns BOOL, so please compare to FALSE instead of 0. I know it's the same, but let's keep it cleaner :)

@yuya-oc

This comment has been minimized.

Contributor

yuya-oc commented Sep 22, 2017

Thanks! I forgot them when updating my changes.

@alespergl alespergl merged commit 2abde14 into electron:master Sep 25, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@yuya-oc yuya-oc deleted the yuya-oc:win7-notification-duration branch Sep 26, 2017

@yuya-oc yuya-oc referenced this pull request Feb 15, 2018

Merged

Upgrade Electron #711

3 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment