Fixes "event.sender.send is not a function"#1214
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
|
Please add a meaningful self-descriptive name to the PR. |
|
That's a better name. It's good to have PR titles that give us an idea of what's being fixed just by reading it. The numbers don't mean anything to a reader |
|
|
||
| contextBridge.exposeInMainWorld( | ||
| 'rendererApi', rendererApi | ||
| ); |
| send: (channel, value) => | ||
| for (let i = 0; i < 2; i++) | ||
| { | ||
| $('body').append(`<input id="leave-by" value="${i}${i}:${i}${i}" />`); |
There was a problem hiding this comment.
If this is being appended inside the for-loop, I imagine the body ends up with 2 leave-by elements as the cleanup happens only before it()
dc4df85 to
5e771d7
Compare
|
@araujoarthur0 will you please take a look? :) |
| * Validates that a string is a valid time, following the format of HH:MM | ||
| * @returns true if it's valid | ||
| */ | ||
| static validateTime(time) |
There was a problem hiding this comment.
Could this function have been used in searchLeaveByElement?
There was a problem hiding this comment.
Yes! I don't know why I didn't consider that, good call.
| const timeDate = new Date(); | ||
| timeDate.setHours(Number(hours), Number(minutes), 0, 0); | ||
| const localizedTime = timeDate.toLocaleTimeString(undefined, { hour: 'numeric', minute: '2-digit' }); | ||
| const message = i18NextConfig.getCurrentTranslation('$Notification.time-to-leave', { time: localizedTime }); |
There was a problem hiding this comment.
Does this message have a placeholder where time gets used? I didn't see one in the English file.
araujoarthur0
left a comment
araujoarthur0
left a comment
There was a problem hiding this comment.
Looking good, just some small items
modified: __tests__/__renderer__/notification-channel.mjs modified: js/notification.mjs modified: js/time-math.mjs modified: renderer/notification-channel.js modified: renderer/preload-scripts/renderer-api.mjs
Signed-off-by: Casey Parker <02-germane-atomic@icloud.com>
modified: __tests__/__renderer__/notification-channel.mjs modified: js/notification.mjs modified: js/time-math.mjs modified: renderer/notification-channel.js modified: renderer/preload-scripts/renderer-api.mjs
775d1d2 to
dc345ee
Compare
|
@araujoarthur0 are you ok with the current revision? |
|
\changelog-update |
3 participants
Related issue
Closes #1162
Context / Background
This one has been an issue for a long time now. Here's what I've figured out. The whole time, we've been trying to use sender.send, which is part of the main process. What we needed to be using was the ipcRenderer.send, but that presented its own issues. It turns out a change in the preload js allows ipcRenderer.send to work.
What change is being introduced by this PR?
How did you approach this problem?
What changes did you make to achieve the goal?
What are the indirect and direct consequences of the change?
The notifications work now, and the
event.sender.send is not a functionis gone.How will this be tested?
Standard tests should pass and the method for reproducing #1162 should fail.