bbc / simorgh

The BBC's Open Source Web Application. Contributions welcome! Used on some of our biggest websites, e.g.
https://www.bbc.com/pidgin
Other
1.42k stars 228 forks source link

Add error boundary component; agree and document when to use it #211

Closed jamesbhobbs closed 5 years ago

jamesbhobbs commented 6 years ago

Is your feature request related to a problem? Please describe. Error Boundaries catch render errors and prevent a total render failure on the page, they can also show fallback components. Docs: https://reactjs.org/docs/error-boundaries.html

Describe the solution you'd like Create a component. Initially simplistic, which renders a basic fallback message saying 'This component has failed to render' - UX and product is very welcome to change this copy.

We must not do this on all components, only on components that our UX and product teams validate can be rendered in spite of an error. i.e. it must never be used as an overall best-efforts rendering solution. There are a huge number of possible uses, a general pattern should not be agreed. It should be a core UX consideration in each and every case - IMO it is as fundamental as progressive enhancement.

Describe alternatives you've considered A HOC, see: https://twitter.com/dan_abramov/status/890716011133100032?lang=en for a nice discussion of when to and not to use it.

Don't do it at all for fear of overuse.

Additional context A small example would be caption within a figure errors, it seems highly likely that the desired UX would be to still render the figure, but clearly state in the caption that it has failed to render. This would avoid reputation damage and still give content.

NB accessibility only errors would be able to be rendered in this error case. For example page component x has a child component y for only screen reader enhancement (probably not a real example), if UX and product want it x can have an error boundary around y so that x can render if y errors despite this causing an accessibility issue. This is because it's unreasonable to expect errors for a subset of users to stop all other users accessing the content in the rest of the page. This does not in any way mean that this is an acceptable way of bypassing accessibility standards. Any errors caught by the error boundaries should be fixed, and accessibility errors are a P1 incident (BBC parlance for must fix ASAP) FYI @greenc05

greenc05 commented 6 years ago

Sounds good @benjaminhobbs

ChrisBAshton commented 5 years ago

After a product discussion with Jim, closing this in favour of #1099 and #1100.