OfficeDev / office-js-helpers

[ARCHIVED] A collection of helpers to simplify development of Office Add-ins & Microsoft Teams Tabs
MIT License
126 stars 56 forks source link

Fix UI.notify with primitive strings #75

Closed casieber closed 6 years ago

casieber commented 6 years ago

UI.notify was using if (body instanceof String) to check if we had a string, but that only checks for the String type and not primitive strings (which is what you get with const x = 'hello';). This was causing a runtime error when you tried UI.notify('Please notify me!'); and would only work with UI.notify(new String('Please notify me!'));. I've done the following to address the issue:

  1. Added typeof body === 'string' to the if check to handle string primitives
  2. Added a test file for UI that contains tests for the param parsing
  3. (At the request of @Zlatkovsky) extended notify to accept a message type of any and the implementation attempts to handle it as best it can. This will allow for things like UI.notify(5); to succeed and be accepted by the type system.

Fixes #74.

Zlatkovsky commented 6 years ago

@casieber , let me know when you want me to merge in (i.e., whether you want to still tweak the error message handling, or just use it as is)

casieber commented 6 years ago

@Zlatkovsky I've tweaked it to better handle the "[object Object]" case using some of the snippet you sent over. Should be good to go now if you approve.

Zlatkovsky commented 6 years ago

Awesome, thanks Colby!