Use new FSM payload and JSON editor to build message

onmyway133 / PushNotifications

🐉 A macOS, Linux, Windows app to test push notifications on iOS and Android

https://onmyway133.com

Other

2.26k stars 176 forks source link

Use new FSM payload and JSON editor to build message #47

Closed clemdesign closed 3 years ago

clemdesign commented 3 years ago

Hi,

First thanks for your great tool !

I propose the following modifications:

Use new FSM notification payload including notification field and data field
Use the following JSON editor to build message: https://github.com/AndrewRedican/react-json-editor-ajrm

I tested the tool for iOS and Android and works.

Thank you for considering this improvment :)

Best !

Ariandr commented 3 years ago

Hi @clemdesign Thanks for your PR 👍 I looked into the changes, they seem okay. And they are mainly for Android. For iOS you've just changed the editor, right?

I believe @onmyway133 should also review the changes and if everything is approved we will merge it and create a new release.

Ariandr commented 3 years ago

Also, before creating a new release I will test iOS part on my side just to be sure, since ~90%+ of users use this tool for sending iOS notifications.

onmyway133 commented 3 years ago

@clemdesign thanks for the PR, and thanks @Ariandr for reviewing. I just check the code, looks good, but I need to check and run it first before I can merge.

clemdesign commented 3 years ago

@Ariandr , yes, changes concern mainly Android (FSM and editor). On iOS, changes concern only the editor. Yeah, to test notifications I used this tool for iOS and Postman for Android, but I think that if the same tool can be used for both platforms, it is better.

@onmyway133 Okay :)

onmyway133 commented 3 years ago

Hei, I take a look and it seems OK. The new editor seems fancy but one thing I'm a bit worried is it has some reported issues https://github.com/AndrewRedican/react-json-editor-ajrm/issues

But overall, great PR @clemdesign 👍

clemdesign commented 3 years ago

Oh you're right about some reported issues. I focused on making friendly the editor that I forgot to check issues.
I did some test (edition, copy, paste...) and works but maybe for certain situation, the plugin does not work.
So, I think possibilities are :

not embed the new editor
add option to offer possibility to use TextPlain editor or JSON editor
embed the JSON editor and trust that for the use of the tool, it is acceptable

onmyway133 commented 3 years ago

@clemdesign for now it looks OK, I'm fine with going with it. If it has many issues then we can remove it later.

Ariandr commented 3 years ago

I will create a release with the updates.

Ariandr commented 3 years ago

New release: https://github.com/onmyway133/PushNotifications/releases/tag/1.8.0

Ariandr commented 3 years ago

@clemdesign @onmyway133 I deleted the release. I started doing some specific activities and found it difficult to fight with this JSON editor. It would constantly jump on the first line and show an error even when I want to insert a simple string.