braze-inc / braze-web-sdk

Public repo for the Braze Web SDK
https://www.braze.com
Other
73 stars 26 forks source link

Documentation Problem: InAppMessage.buttons default #16

Closed blimmer closed 7 years ago

blimmer commented 7 years ago

The documentation indicates that the default param is an empty array, but it appears to be undefined.

See https://github.com/blimmer/ember-appboy/issues/35 .

blimmer commented 7 years ago

Actually, this might be our fault because of how we're overriding showInAppMessages to handle the transition within the context of our Single Page App framework. I'm going to close this, but feel free to reopen if I'm wrong.

See also this PR https://github.com/blimmer/ember-appboy/pull/36

froodian commented 7 years ago

No worries @blimmer, I think your second explanation is correct, as our implementation of showInAppMessage does start with protection against potentially being given a null inAppMessage param, and in the constructor of ab.InAppMessage we set this.buttons to [] if it's null/undefined. Note also though that the inAppMessage param for showInAppMessage could be a ab.ControlMessage (in which scenario you should still call the base implementation) so even if it's not null you should defend against inAppMessage.buttons potentially being undefined. I think this may actually be the root cause of an issue one of your users is seeing where control message enrollments aren't getting logged (because the base implementation of showInAppMessage isn't getting called for them). Thanks for your work, feel free continue to to ask more questions and file issues as they arise!

blimmer commented 7 years ago

Awesome - thank you for the fast follow up, as always @froodian . I just confirmed that everything is working with a control message being passed. I published a bugfix version of ember-appboy resolving this issue.