muan
commented
4 years ago Thanks for the pull request.
For our custom elements we aim to be as simple as possible, which is why we provide events as an interface between the elements and app code so developers have the bits they need to build on top of it.
Setting error HTML is a very app specific behavior that can open doors for further customization (eg. interpolation, multiple error state, both had been requested in other elements) that we'd rather not add/maintain.
If we were to add this behavior, the first thing I'd suggest is we should not be reading HTML from an attribute. HTML should either be already rendered in a page (hidden) which we can target with a data attribute, or be a template element that gets cloned. With that in mind, we already have many precedents for these kind of behaviors being handled on the app site, like include-fragment [data-show-on-error], clipboard-copy[data-copy-feedback], auto-check.is-loading.
In the same vein, we recently removed the HTML setting and error handling responsibility from the auto-check element because of the complexity it has added to the element (https://github.com/github/auto-check-element/issues/17#issuecomment-542459769).
I hope this makes sense. I'd be happy to review a shared behavior like include-fragment [data-show-on-error] being add in github/github.
This change adds an optional parameter to let the user define an error UI. There are existing events the user could hook into but using a parameter seems like an easier path for a developer to support the error state.
I am very open to feedback. Thank you!