Just installed the component locally and found a few issues:
[x] n-ui-foundations not installed in bower.json
[x] the first time the button is clicked the text doesn't update. Maybe related to on line 48 of Newsletter.js you need to return? (Be careful to return promises from functions - sometimes things will basically work if you don't, but it can lead to unpredictability and hard to track down bugs)
[x] I would rename the client directory to js. Client can have 2 meanings - all the client-side code (i.e. everything that gets loaded in the browser), which would suggest the styles should be in there too; or a library to communicate with an API. Neither of those descriptions match what is actually in there at the moment
I wonder if the implementation could be simplified. Do you have any ideas on this? I wonder if it'd be simpler to reason about if feedback and newsletter classes were combined into one (Just something to think about, not to actually do anything about )
[ ] Check that sensible tracking events are being sent (unfortunately the demo doesn't have tracking set up... but this is probably a general problem with all our demos). To check this try it out in the article app, and look for the ingest events in the network tab in dev tools
Just installed the component locally and found a few issues:
return
? (Be careful to return promises from functions - sometimes things will basically work if you don't, but it can lead to unpredictability and hard to track down bugs)client
directory tojs
. Client can have 2 meanings - all the client-side code (i.e. everything that gets loaded in the browser), which would suggest the styles should be in there too; or a library to communicate with an API. Neither of those descriptions match what is actually in there at the momentingest
events in the network tab in dev tools