Display row-level errors in instance editor - #1476
Conversation
epatters
added
enhancement
a1bbdae to
a27acfe
Compare
| const firstRow = canvas.getAllByRole("row")[1]!; | ||
| const cell = within(firstRow).getAllByRole("gridcell")[activeColumn]!; | ||
| const firstRow = canvas.getAllByRole("row")[1] as HTMLTableRowElement; | ||
| const cell = firstRow.cells[activeColumn]!; |
There was a problem hiding this comment.
I don't want to complain about having tests for UI components, but these are rather fragile.
There was a problem hiding this comment.
That's fair, I'll prune these and look at making them less brittle.
| undefined | ||
| cellIssues(cell()) | ||
| .map(issueMessage) | ||
| .join("\n") || undefined |
There was a problem hiding this comment.
It would be good to improve the styling of these too. I could do that in a follow-up PR.
kasbah
left a comment
kasbah
left a comment
There was a problem hiding this comment.
Just UI wise I'd prefer highlighting (background) the whole row in the same pink/red we are using for the other errors. I guess then the tooltip needs to appear on the whole row as well. I'd be ok with changing that on a follow-up PR though. Rest LGTM.
a27acfe to
c65c4e5
Compare
|
Thanks for the review! I actually started by coloring the whole row red, but thought it looked too aggressive, as well as possibly being confusing with cell-level errors. Anyway, we can play around with it. |
3 participants
The presence of errors is indicated by coloring the new row headers red and the errors themselves are displayed on hover in a tooltip.