We want to show staff alerts also in other places in the app so I factored it out to a hook in another file.
Related to: https://linear.app/comm/issue/ENG-9807/add-staff-alert-for-decryption-related-errors
Details
Details
- Reviewers
kamil tomek ashoat - Commits
- rCOMM7efc7eda5e27: [lib] Factor out useStaffAlert()
Run the app and see if alerts are shown.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
Thanks for doing this!
lib/shared/staff-utils.js | ||
---|---|---|
2 ↗ | (On Diff #45393) | Nit: please maintain the newline after the // @flow declaration |
52–57 ↗ | (On Diff #45393) | Not sure how useful this one is... I would just use JSON.stringify at the callsite instead of requiring this generic utility to be aware of DMOperation |
Comment Actions
This implementation has a performance consequence - in the original implementation, we were only stringifying when an alert was shown. Now we're stringifying the object even for users who will never see an alert. We could consider keeping the JSON.stringify inside showAlertToStaff, but I'm not sure if it is really that important.
Comment Actions
var startTime = performance.now() for(var i = 0; i < 1000; ++i) { JSON.stringify({"asdfasdf": "fhsdjfhsjdhf", "fjksdfhksdhfj": 234234}) } var endTime = performance.now() console.log(`Call to doSomething took ${endTime - startTime} milliseconds`) VM529:8 Call to doSomething took 0.5 milliseconds
I guess we can live with it ;) I'm landing this.