adding the setNotifications function

[filipeotero]

In this PR, instead of requesting the back-end API, we insert the notifications as a parameter to the notifications panel.
Also, the notifications-panel is updated according to dynamods organization.

FYI: @QilongTang

[QilongTang]

@filipeotero It looks like after your change, launching the webApp alone, folks can no longer test the display of message from our backend endpoint. Anyway we can preserve both options?

[filipeotero]

@filipeotero It looks like after your change, launching the webApp alone, folks can no longer test the display of message from our backend endpoint. Anyway we can preserve both options?

We can set an env variable that gets the notifications by itself or waits to get the notifications from the setNotifications function

[QilongTang]

This is still behind

[QilongTang]

👍🏻

[filipeotero]

Hi @QilongTang, notice that in this new commit I added a verification before fulfilling the notifications list. If the env URL is empty, we set the setNotifications function from outside the webApp. Otherwise, we grab the notifications by the backend.

[QilongTang]

Hi @QilongTang, notice that in this new commit I added a verification before fulfilling the notifications list. If the env URL is empty, we set the setNotifications function from outside the webApp. Otherwise, we grab the notifications by the backend.

In this case, you probably want to move the value in https://github.com/DynamoDS/NotificationCenter/blob/master/config/.env to .env.dev so that when this package is bundled in prod and used inside Dynamo, the WebApp is used for purely displaying notifications passed in by Dynamo. But when building it locally, it will grab notifications from dev end point. What do you think @zeusongit ?

[avidit]

In this case, you probably want to move the value in https://github.com/DynamoDS/NotificationCenter/blob/master/config/.env to .env.dev so that when this package is bundled in prod and used inside Dynamo, the WebApp is used for purely displaying notifications passed in by Dynamo. But when building it locally, it will grab notifications from dev end point. What do you think @zeusongit ?

@QilongTang I have made these changes and updated package.json with --env parameters in https://github.com/DynamoDS/NotificationCenter/pull/23/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R24-R25

[zeusongit]

Hi @QilongTang, notice that in this new commit I added a verification before fulfilling the notifications list. If the env URL is empty, we set the setNotifications function from outside the webApp. Otherwise, we grab the notifications by the backend.

In this case, you probably want to move the value in https://github.com/DynamoDS/NotificationCenter/blob/master/config/.env to .env.dev so that when this package is bundled in prod and used inside Dynamo, the WebApp is used for purely displaying notifications passed in by Dynamo. But when building it locally, it will grab notifications from dev end point. What do you think @zeusongit ?

Makes sense, with @avidit's changes we will get different URL's based on the environment used.

[QilongTang]

Thanks, with the other PR in progress, is this one good to go? @zeusongit @avidit

[zeusongit]

Looks good, just one comment, maybe unrelated to this PR, but we include this script tag:

NotificationCenter/src/index.html

Line 13 in 7182623

<script src="index.bundle.js"></script>

which is understandable if we are working on notification center alone but inside dynamo mainJs is replaced by the same file we are including from the script tag, therefore are we including the bundle file twice, when running inside Dynamo?

[QilongTang]

@zeusongit Ah this is something I forgot to mention to you, when launching in Dynamo, <script src="index.bundle.js"></script> will fail to load. This only works for dev build here in this repo. Maybe we should use different html for differernt build instead of mixing things..