-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
fix(@jest/reporters): move missing icon file which is needed for NotifyReporter
class
#12593
Conversation
do you allow notification for |
@F3n67u Yep. For some reason this was disabled by default. I see the notifications, but Jest icon is not there (see screenshots in #5898). I was playing with examples from |
I cannot show icons for notification either. I am using the underly
I think this doc will be related, maybe when macOS upgrade the private method which
from https://github.com/julienXX/terminal-notifier readme. UPDATE: there is a related bug report julienXX/terminal-notifier#283 which reports appIcon is not working on big sur. we could move our discussion there. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
Ah.. The icon issue on macOS is documented here: https://github.com/mikaelbr/node-notifier#macos-custom-icon-without-terminal-icon |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
Found accidentally.
NotifyReporter
class from@jest/reporters
is referencing'../assets/jest_logo.png'
, but this file actually lives inside@jest/core
.The icon was added for
node-notifier
in #1170. For some reason I cannot make it work on MacOS. Might be it worked withgrowl
(screenshots can be found in #5898). Perhaps it shows up on Windows?Seems like
jest-cli
was massive. No wonder that one file got lost. Reporters were split into separate package in #7902, but the icon was moved later to@jest/core
in #7696.Does this make senses? (;
Test plan
Green CI.