Skip to content

Update README.md#83

Merged
iMattPro merged 6 commits into
mainfrom
iMattPro-patch-1
Oct 1, 2024
Merged

Update README.md#83
iMattPro merged 6 commits into
mainfrom
iMattPro-patch-1

Conversation

@iMattPro

@iMattPro iMattPro commented Oct 1, 2024

Copy link
Copy Markdown
Contributor

No description provided.

@iMattPro
iMattPro requested a review from rxu October 1, 2024 15:12
Signed-off-by: Matt Friedman <maf675@gmail.com>
Comment thread README.md Outdated
4. **Caveats for Local Testing**
- Local testing of Push Notifications only works from a `localhost` address or if your local server has a secure SSL certificate.
- We have seen success on Windows using manually installed PHP, Apache and MySQL. However, for reasons not yet known we do not see success on Mac using MAMP.
- Depending on your local server's setup, operating system, and browser versions, it is still possible that testing push notifications may not work (for example, in a local environment running on macOS, only Chrome will show notifications while it is open or after it has been launched).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd also add 2 things:

  • notifications should be allowed for both browser applications in respective operation system,
  • 2nd browser should be open or working in a background (sometimes, because if you close it OS may terminate the background process in some cases) to be able to display notifications.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

2nd notification isn't necessary as we're only using 1 account to recieve and another account to trigger. Want to keep it simple and not confusing.

@rxu rxu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Otherwise LGTM.

Signed-off-by: Matt Friedman <maf675@gmail.com>
@iMattPro
iMattPro merged commit 8960d27 into main Oct 1, 2024
@iMattPro
iMattPro deleted the iMattPro-patch-1 branch October 1, 2024 20:20
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