feat(admin): adding uniformity in cancel and back buttons present in FIDO, SMTP & Services#2409
feat(admin): adding uniformity in cancel and back buttons present in FIDO, SMTP & Services#2409
Conversation
…i-issue-2361-scim
…Jans Lock (#2405) * feat(admin): adding uniformity in cancel and back buttons present in Jans Lock * feat(admin): adding uniformity in cancel and back buttons present in Jans Lock * Code Rabbit fixes
✅ Actions performedReview triggered.
|
1 similar comment
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx(3 hunks)admin-ui/app/routes/Apps/Gluu/GluuFormFooter.tsx(1 hunks)admin-ui/plugins/fido/components/DynamicConfiguration.tsx(5 hunks)admin-ui/plugins/fido/components/StaticConfiguration.tsx(14 hunks)admin-ui/plugins/scim/components/ScimConfiguration.tsx(1 hunks)admin-ui/plugins/services/Components/Configuration/CachePage.js(2 hunks)admin-ui/plugins/services/Components/Configuration/PersistenceDetail.js(2 hunks)admin-ui/plugins/smtp-management/components/SmtpManagement/SmtpForm.tsx(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.926Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
📚 Learning: 2025-11-03T08:47:33.926Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.926Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
Applied to files:
admin-ui/plugins/services/Components/Configuration/PersistenceDetail.jsadmin-ui/plugins/fido/components/DynamicConfiguration.tsxadmin-ui/app/routes/Apps/Gluu/GluuFormFooter.tsxadmin-ui/plugins/services/Components/Configuration/CachePage.js
🔇 Additional comments (11)
admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx (1)
53-61: LGTM! Modal reset logic improved.The use of
useRefto track the previous modal state ensures thatuserMessageis reset only when the modal transitions from closed to open, preventing unnecessary resets on every render. This is a solid improvement.admin-ui/app/routes/Apps/Gluu/GluuFormFooter.tsx (1)
56-56: LGTM! Sensible default value.Setting
disableBack = falseas the default provides a more convenient API while maintaining backward compatibility with existing usages.Based on learnings.
admin-ui/plugins/scim/components/ScimConfiguration.tsx (1)
91-91: LGTM! Defensive boolean handling.Using the nullish coalescing operator ensures that
isLoadingalways receives a boolean value, even ifisSubmittingisundefinedornull.admin-ui/plugins/services/Components/Configuration/PersistenceDetail.js (1)
126-128: LGTM! Appropriate footer for read-only page.Since this is a read-only database information page, showing only the Back button without Cancel or Apply is the correct choice.
Based on learnings.
admin-ui/plugins/fido/components/DynamicConfiguration.tsx (2)
308-319: LGTM! Double-toggle issue resolved.The previous critical issue has been fixed correctly. With
applyButtonType="button"andonApply={toggle}, the Apply button only opens the confirmation dialog without submitting the form. The actual submission occurs through theGluuCommitDialog'sonAccepthandler.
33-33: Verify the impact of validateOnMount.Enabling
validateOnMount: truemeans the form will show validation errors immediately on page load before the user interacts with any field. This could be jarring UX if required fields start showing errors right away.Please confirm this is intentional and aligns with the desired user experience. If the intent is to enable the Apply button based on validity, consider using
validateOnChangeandvalidateOnBlurinstead, or ensure your validation schema doesn't mark untouched fields as errors.admin-ui/plugins/smtp-management/components/SmtpManagement/SmtpForm.tsx (2)
71-80: LGTM! Robust validation before submission.The async validation logic in
submitFormproperly validates the form, marks all invalid fields as touched, and displays a user-friendly error message before allowing submission. This prevents incomplete data from being submitted.
58-58: Verify the impact of validateOnMount.Enabling
validateOnMount: truewill show validation errors immediately when the form loads, which may negatively impact user experience if required fields display errors before any user interaction.Please confirm this behavior is intentional and consistent with UX requirements across the admin UI. Consider whether validation should only trigger on user interaction (
validateOnChange/validateOnBlur) instead.admin-ui/plugins/fido/components/StaticConfiguration.tsx (3)
368-379: LGTM! Double-toggle issue resolved.The previous critical issue has been addressed correctly. The Apply button uses
applyButtonType="button"withonApply={toggle}, ensuring the confirmation dialog opens without auto-submitting the form.
95-118: LGTM! Metadata servers transformation wrapper implemented correctly.The
metadataServersFormikwrapper elegantly handles the bidirectional transformation between{key, value}(used by GluuProperties) and{url, rootCert}(used in form values). ThesetFieldValuewrapper correctly passes through theshouldValidateflag to maintain Formik's validation behavior.The
useMemodependencies are appropriate -formik.valuesandformik.setFieldValueare the actual dependencies used within the memoized computation.
54-54: Verify the impact of validateOnMount.Setting
validateOnMount: truewill immediately show validation errors when the form loads, potentially before the user has interacted with any field. This could result in a poor user experience if required fields are flagged as invalid from the start.Please confirm this is the intended behavior and aligns with UX standards across the admin UI. If the goal is to control button enablement, consider whether
validateOnChangeandvalidateOnBlurwould provide a better user experience.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/services/Components/Configuration/CachePage.js(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.926Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
📚 Learning: 2025-11-03T08:47:33.926Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.926Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
Applied to files:
admin-ui/plugins/services/Components/Configuration/CachePage.js
🔇 Additional comments (2)
admin-ui/plugins/services/Components/Configuration/CachePage.js (2)
5-5: LGTM!The import statement correctly updates to
GluuFormFooterto support the new footer component migration.
238-241: Modal close timing is now correct.The previous issue regarding modal close timing has been properly addressed. The modal now closes before submission begins, providing better UX.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Signed-off-by: Faisal Siddique <71010439+faisalsiddique4400@users.noreply.github.com>
|
|
3 participants
feat(admin): adding uniformity in cancel and back buttons present in FIDO & SMTP & SERVICES
Summary
This update introduces consistent behavior for Cancel, Back, and Apply buttons across Admin UI forms, including correct enabling and disabling based on form state.
Improvements
Additional Enhancements
Affected Modules
This change is applied across all forms where configuration changes are possible, including but not limited to:
Parent issue: #2268
Testing Steps
Summary by CodeRabbit