-
Notifications
You must be signed in to change notification settings - Fork 6
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
Remove login buttons when the submission limit is reached #793
base: main
Are you sure you want to change the base?
Remove login buttons when the submission limit is reached #793
Conversation
Bundle ReportChanges will increase total bundle size by 72 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: @open-formulieren/sdk-esmAssets Changed:
Files in
view changes for bundle: @open-formulieren/sdk-OpenForms-umdAssets Changed:
Files in
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #793 +/- ##
==========================================
- Coverage 84.23% 83.86% -0.37%
==========================================
Files 245 245
Lines 4808 4810 +2
Branches 1279 1274 -5
==========================================
- Hits 4050 4034 -16
- Misses 729 747 +18
Partials 29 29
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Not exactly sure how the coverage has decreased from this change. Any ideas? |
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 don't know either what's wrong with the coverage.
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.
Could you place this logic outside the conditional rendering? Chaining if statements is something we should try to avoid. I think this also makes it a bit more clear why things are happening
Maybe something like:
const showStartButton = !hasActiveSubmission && !form.submissionLimitReached;
...
{hasActiveSubmission && (
<ExistingSubmissionOptions
form={form}
onDestroySession={onDestroySession}
isAuthenticated={isAuthenticated}
/>
)}
{showStartButton && (
<LoginOptions
// if cosign allows links in emails, we don't need to display the cosign
// login options, so strip them out
form={form.cosignHasLinkInEmail ? {...form, cosignLoginOptions: []} : form}
onFormStart={onFormStart}
extraNextParams={extraNextParams}
/>
)}
78fbd8d
to
db553bf
Compare
…sion limit is reached Doesn't make much sense to show these buttons if users are unable to start a form because the submission limit has been reached.
db553bf
to
3974f79
Compare
Closes open-formulieren/open-forms#5046