Scaffolder - Addition of Dismissable Error Banner #13522
Scaffolder - Addition of Dismissable Error Banner #13522freben merged 3 commits intobackstage:masterfrom
Conversation
Signed-off-by: mufaddal motiwala <mufaddalmm.52@gmail.com>
Changed Packages
|
Signed-off-by: mufaddal motiwala <mufaddalmm.52@gmail.com>
| }; | ||
|
|
||
| export const TaskErrors = ({ taskStream }: TaskPageLinksProps) => { | ||
| return taskStream.error ? ( |
There was a problem hiding this comment.
Does this really work? Seems to me that the taskStream itself doesn't necessarily have its reference (instance) value change? Since the error is what actually "drives" redraws here, it seems to me that if this works then it's accidentally because the surrounding component had to redraw itself, right? Essentially I'm asking if there should be a singe error?: Error | undefined prop instead
There was a problem hiding this comment.
It does work, I checked it on fresh instances.
So I chose to take the entire TaskStream Prop as in the future we may be extracting errors from EventStream stepLogs , yeah for now we can only take a single prop, should I make that change?
| return taskStream.error ? ( | ||
| <Box> | ||
| <DismissableBanner | ||
| id={String(Math.random())} |
There was a problem hiding this comment.
This will update the id on every repaint, which probably is not desirable. The render phase should be side-effect-free. Maybe you could have something like this (untested written out of my own head):
const id = useRef('');
useEffect(() => {
id.current = String(Math.random());
}, [error]);and then use id={id.current}
There was a problem hiding this comment.
This is much better, thanks!
Signed-off-by: mufaddal motiwala mufaddalmm.52@gmail.com
Hey, I just made a Pull Request!
This PR is our first step to enhance exception handling in the Scaffolder and show it back to the user.
Currently I have made a simple change and have managed to show the user an error pop up , whenever an error is getting logged in the completion phase using the existing
TaskStreamfeatures.Future
The future plan is to enhance exception handling by adding the Error field to
TaskEventType, something like this -export type TaskEventType = 'completion' | 'log' | 'error';#12760