Skip to content

Conversation

miniak
Copy link
Contributor

@miniak miniak commented Jul 18, 2019

Description of Change

According to the NIIF_LARGE_ICON documentation:

NIIF_LARGE_ICON (0x00000020)
0x00000020. Windows Vista and later. The large version of the icon should be used as the notification icon. This corresponds to the icon with dimensions SM_CXICON x SM_CYICON. If this flag is not set, the icon with dimensions SM_CXSMICON x SM_CYSMICON is used.

The other option would be to keep the SM_CXSMICON size, but to drop the NIIF_LARGE_ICON flag.

Checklist

Release Notes

Notes: Fixed tray.displayBalloon() not working with custom icon on Windows.

@miniak miniak added the wip ⚒ label Jul 18, 2019
@miniak miniak self-assigned this Jul 18, 2019
@electron-cation electron-cation bot added new-pr 🌱 PR opened recently and removed new-pr 🌱 PR opened recently labels Jul 18, 2019
@miniak miniak removed the wip ⚒ label Jul 23, 2019
@miniak miniak force-pushed the miniak/tray-balloon-icon branch from aa43366 to ddf52bf Compare July 23, 2019 07:11
@miniak miniak marked this pull request as ready for review July 23, 2019 07:11
@miniak miniak requested a review from codebytere July 23, 2019 07:11
@miniak miniak requested a review from deermichel July 23, 2019 07:54
@deermichel
Copy link
Contributor

Which code did you use to test this? Doesn't seem to work for me 🤔

@miniak
Copy link
Contributor Author

miniak commented Jul 23, 2019

@deermichel just this snippet in Fiddle

const icon = nativeImage.createFromPath('C:\\Windows\\WinSxS\\amd64_microsoft-windows-dxp-deviceexperience_31bf3856ad364e35_10.0.18362.1_none_19829bbf76ffad81\\ringtones.ico')
const tray = new Tray(icon)

tray.on('click', () => {
  tray.displayBalloon({
    title: 'Hello World!',
    content: 'Lorem Ipsum',
    icon
  })
})

@miniak miniak requested a review from felixrieseberg July 24, 2019 14:05
Copy link
Contributor

@deermichel deermichel left a comment

Choose a reason for hiding this comment

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

Alright, confirmed ✅

Copy link
Member

@codebytere codebytere left a comment

Choose a reason for hiding this comment

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

approving in agreement with issue description and confirmation by @deermichel of change efficacy.

@alexeykuzmin alexeykuzmin merged commit 9ab3ec0 into master Jul 30, 2019
@release-clerk
Copy link

release-clerk bot commented Jul 30, 2019

Release Notes Persisted

Fixed tray.displayBalloon() not working with custom icon on Windows.

@trop
Copy link
Contributor

trop bot commented Jul 30, 2019

I have automatically backported this PR to "6-0-x", please check out #19528

@trop
Copy link
Contributor

trop bot commented Jul 30, 2019

I have automatically backported this PR to "4-2-x", please check out #19529

@trop
Copy link
Contributor

trop bot commented Jul 30, 2019

I have automatically backported this PR to "5-0-x", please check out #19530

@trop trop bot removed target/6-0-x labels Jul 30, 2019
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.

4 participants