-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix DevTools in subform #2819
base: main
Are you sure you want to change the base?
Fix DevTools in subform #2819
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! 🙌
const hasPresentation = useHasPresentation(); | ||
|
||
if (overriddenDataModelUuid) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of these task-store checks in the loader was something we added to avoid the "page is loading" for each subform during the PDF render.
Have you tested how this now looks during PDF generation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Also counts for summaries of subforms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, will check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The summary table looks good, but in PDF it looks weird indeed. Good catch. What component is used here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless i am missing something @danielskovli
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bjosttveit @olemartinorg Last time I was involved in this part of the code, the ready-for-print mechanism waited for all subform data to finish loading 👌
The Loader modifications were only visual if I remember correctly. Worth noting that the overriddenTaskId
check was already in there, I think perhaps @adamhaeger had added it as part of the Summary2 feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overriddenTaskId
is (or was?) needed to render summary of a previous task.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the new skeleton loader 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still is, this will just show a loader instead of null while it loads. Due to useHasPresentation
it should just show the loader and not the Presentation
container around it. In PDF however we don't have any Presentation
container, so I added a DummyPresentation
that only sets the provider so that loading will not create its own Presentation
there either
Quality Gate failedFailed conditions |
Description
Fixes dev-tools crashing in subform. Also makes the subform-loading look a bit better by not showing an empty form container.
Added an error boundary inside of the dev tools panel as well, so errors like this are less annoying in the future
Cleaned up dev tools component by removing
{children}
since we no longer wrap the application in it.Related Issue(s)
Verification/QA
kind/*
label to this PR for proper release notes grouping