feat: add skeleton error states #727
Conversation
ryanomackey
requested review from
natashadecoste,
theiliad and
a team
as code owners
ryanomackey
force-pushed
the
feat/error-states
branch
from
8732bec
to
af4f9e5
Compare
There was a problem hiding this comment.
when i hover on orange status icon, it turns purple 🟣
you can see it in: https://deploy-preview-727--carbon-charts-core.netlify.app/?path=/story/line--line-error
Ha. I almost hate to fix that. |
There was a problem hiding this comment.
mainly for me the icon is standing out. it isn't the exclamation carbon-icon which is a problem. @natashadecoste could you assist @ryanomackey here with how you used that icon in MeterChart?
| loading: false, | ||
| error: { | ||
| title: "No data available", | ||
| subtitle: "Lorem ipsum dolor sit." |
There was a problem hiding this comment.
Maybe we should use a real subtitle here to make for a better demo? e.g. Unable to reach application servers at this time
| @@ -422,3 +422,29 @@ export const lineSkeletonOptions = { | |||
| loading: true | |||
| } | |||
| }; | |||
|
|
|||
| // line - error | |||
| export const lineErrorData = []; | |||
There was a problem hiding this comment.
What happens if data is provided? would it still show the error?
| this.removeSkeleton(); | ||
| this.removeErrorMessage(); | ||
| } | ||
| } |
There was a problem hiding this comment.
shouldn't the error be shown at all times regardless of whether the chart is loading or not? Errors probably won't always just be contained to the data, they could be about anything, and if you don't want an error to show you'd probably just remove it from options...?
|
|
||
| const exclamationPoint = errorContainer | ||
| .selectAll("text.bx--cc--error-message__exclamation-point") | ||
| .data(["!"]); |
There was a problem hiding this comment.
we should be using the correct carbon-icon here rather than an !
| .classed("bx--cc--error-message__subtitle", true) | ||
| .merge(errorSubtitle) | ||
| .attr("x", "2.65em") | ||
| .attr("y", height / 2 + 18) |
| .append("text") | ||
| .classed("bx--cc--error-message__title", true) | ||
| .merge(errorTitle) | ||
| .attr("x", "2.25em") |
| .data([ | ||
| { | ||
| cx: "1em", | ||
| cy: height / 2 - 5, |
|
Also, I'm still seeing the purple hover state in the demos |
…s into feat/error-states
|
Tyler or Jeannie, could you please review? Do we just need to support error states here or should we support other icons & statuses for other types of messages? e.g. |
|
@jeanservaas could you take a look at @theiliad's questions? @ryanomackey could you respond to some of @theiliad's questions? |
I've just msged Jeannie to review. I think overall I'd like to know if we should add support for more icons or other error styling that we might need. Being able to also add an |
|
@theiliad I would def just start with support icons here. Eventually we want to create a more comprehensive set of status icons (there's def and issue somewhere in the carbon backlog) but it hasn't moved forward yet. ps, right now (in Safari) I'm seeing the icons but without the adornments (the exclamation mark)
|
Updates
Demo screenshot or recording
Review checklist (for reviewers only)