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

Add TextNow recipe #416

Closed
wants to merge 3 commits into from
Closed

Add TextNow recipe #416

wants to merge 3 commits into from

Conversation

Arthur-Huan
Copy link
Contributor

Pre-flight Checklist

Please ensure you've completed all of the following.

Description of Change

Adding the recipe for TextNow.

Copy link
Member

@SpecialAro SpecialAro left a comment

Choose a reason for hiding this comment

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

Hi! Thanks for raising this PR 😁

I've made some comments on your code if you care to answer.

Generally, LGTM but I don't have a way of testing the code because I don't have an account on this service.

Comment on lines +2 to +8
class Messenger extends Ferdium {
overrideUserAgent() {
return window.navigator.userAgent
.replaceAll(/(Ferdium|Electron)\/\S+ \([^)]+\)/g, '')
.trim();
}
};
Copy link
Member

Choose a reason for hiding this comment

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

Is this needed? If not, please remove it

Comment on lines +80 to +86
localStorage.setItem(
'_cs_desktopNotifsEnabled',
JSON.stringify({
__t: Date.now(),
__v: true,
}),
);
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed?

Comment on lines +88 to +104
if (typeof Ferdium.onNotify === 'function') {
Ferdium.onNotify(notification => {
if (typeof notification.title !== 'string') {
notification.title =
((notification.title.props || {}).content || [])[0] || 'Messenger';
}

if (typeof notification.options.body !== 'string') {
notification.options.body =
(((notification.options.body || {}).props || {}).content || [])[0] ||
'';
}

return notification;
});
}
};
Copy link
Member

Choose a reason for hiding this comment

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

This code is interesting 🤔

Why do you need it? Did you by any chance noticed that the title or body could be different from string and that would cause an error?

@Arthur-Huan Arthur-Huan closed this Sep 4, 2023
@SpecialAro
Copy link
Member

Hi @Arthur-Huan!

Did you close this PR by accident?

@Arthur-Huan
Copy link
Contributor Author

No, I realized some of the files were the wrong ones. It's kind of a mess on my computer and I must have mixed some files up.

@Arthur-Huan Arthur-Huan deleted the beta branch September 4, 2023 00:51
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.

2 participants