-
Notifications
You must be signed in to change notification settings - Fork 22
feat(admin-ui): revamp Health, License, and Audit Message and Loader components (#2584) #2620
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
Conversation
📝 WalkthroughWalkthroughAdds GluuSpinner and GluuPageContent primitives; extends GluuButton with hover-opacity props; centralizes status/UI constants; introduces commit-message validation; refactors Health, License, Loader, and Commit dialog into theme-aware components; adjusts types, exports, styles, and i18n keys. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant CommitDialog as CommitDialog
participant Validator as Validator
participant Parent as Parent
participant Server as Server
User->>CommitDialog: open dialog, type message
User->>CommitDialog: click Accept
CommitDialog->>Validator: getCommitMessageValidationError(length, t)
Validator-->>CommitDialog: "" or error
alt valid
CommitDialog->>Parent: onAccept(message)
Parent->>Server: submit commit
Server-->>Parent: success/failure
Parent-->>CommitDialog: close or show result
else invalid
CommitDialog-->>User: display validation error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin-ui/app/styles/cardBorderStyles.ts (1)
4-31: 🧹 Nitpick | 🔵 TrivialDerive
gradientPositiontype from constants to prevent drift.
Keeps the type aligned withGRADIENT_POSITIONvalues if they change.♻️ Suggested refactor
+type GradientPosition = typeof GRADIENT_POSITION[keyof typeof GRADIENT_POSITION] + interface CardBorderStyleOptions { isDark: boolean borderRadius?: number | string borderWidth?: string - gradientPosition?: 'top left' | 'top right' | 'bottom left' | 'bottom right' | 'center' + gradientPosition?: GradientPosition ellipseSize?: string } - const gradientPositionMap: Record<string, string> = { + const gradientPositionMap: Record<GradientPosition, string> = { [GRADIENT_POSITION.TOP_RIGHT]: '100% 0%', [GRADIENT_POSITION.TOP_LEFT]: '0% 0%', [GRADIENT_POSITION.BOTTOM_RIGHT]: '100% 100%', [GRADIENT_POSITION.BOTTOM_LEFT]: '0% 100%', [GRADIENT_POSITION.CENTER]: '50% 50%', } - const getGradientPosition = (position: string) => { + const getGradientPosition = (position: GradientPosition) => { return gradientPositionMap[position] || gradientPositionMap[GRADIENT_POSITION.TOP_RIGHT] }
🤖 Fix all issues with AI agents
In `@admin-ui/app/components/GluuSpinner/GluuSpinner.tsx`:
- Line 24: Replace the non-semantic div used for the live status in the
GluuSpinner component with a semantic element (e.g., <output>) to satisfy the
a11y lint: locate the JSX that renders <div className={classes.spinner}
role="status" aria-label={ariaLabel} aria-live="polite" /> inside the
GluuSpinner component and change it to a semantic element while preserving the
className ({classes.spinner}), role="status", aria-label={ariaLabel} and
aria-live="polite" attributes so styling and behavior remain identical.
In `@admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx`:
- Around line 115-116: The overlay and modal container divs currently only
handle mouse clicks; add keyboard accessibility by giving them semantic roles
and focusability and wiring keyboard handlers: set role="button" and
tabIndex={0} on the overlay div that calls the existing closeModal when
Enter/Space is pressed and also closes on Escape; on the modalContainer div add
role="dialog" and tabIndex={-1} (or 0 if it should be focusable) and an
onKeyDown that stops propagation for keyboard events and closes on Escape (reuse
the existing closeModal and existing stopPropagation logic inside the
onKeyDown), and ensure the modal has aria-labelledby pointing to the title by
adding id="commit-dialog-title" to the GluuText title element so the dialog is
properly labeled.
- Around line 75-80: The component currently hardcodes the commit message bounds
(10 and 512) when computing isValid which duplicates the constants in
getCommitMessageValidationError; import and use the shared MIN_LENGTH and
MAX_LENGTH constants (the same ones exported by the module that defines
getCommitMessageValidationError) and replace the literals in the isValid
calculation so isValid = messageLength >= MIN_LENGTH && messageLength <=
MAX_LENGTH, keeping userMessage, isValid, and errorMessageText logic intact.
In `@admin-ui/app/routes/Apps/Gluu/styles/GluuCommitDialog.style.ts`:
- Around line 13-123: The modal uses fixed sizes (MODAL_WIDTH, MODAL_HEIGHT,
CONTENT_WIDTH) causing overflow on small screens; update useStyles to make
modalContainer and textareaContainer responsive by replacing fixed width/height
with responsive rules (e.g., width: `min(${MODAL_WIDTH}px, 90%)`, maxWidth:
`${MODAL_WIDTH}px`, height: `min(${MODAL_HEIGHT}px, 90vh)` or auto, and for
textareaContainer use width: `min(${CONTENT_WIDTH}px, calc(100% -
${HORIZONTAL_PADDING * 2}px))` or percentages), add overflow: 'auto' or
maxHeight on modalContainer to allow scrolling on small viewports, and ensure
positioning (transform/left/top) still centers the modal; update constants or
remove where necessary and adjust styles in useStyles for modalContainer,
textareaContainer, title, and textarea to use relative sizing so the dialog
remains usable on narrow screens.
In `@admin-ui/app/routes/License/LicenseDetailsPage.test.tsx`:
- Around line 8-22: The mockLicense test object contains a realistic-looking
licenseKey that triggers secret-scan rules; update the licenseKey property in
the mockLicense object used by LicenseDetailsPage.test.tsx to a clearly
non-secret value (e.g. "TEST_LICENSE_KEY" or "license-key-placeholder") so it
cannot be mistaken for a real key; change only the licenseKey field in the
mockLicense constant to avoid affecting other test data.
In `@admin-ui/app/routes/License/LicenseDetailsPage.tsx`:
- Around line 124-132: In renderLicenseField (the useCallback that renders
LicenseField), replace the falsy-check pattern that uses `field.value || 'N/A'`
with a nullish-coalescing check so valid falsy values (e.g., numeric 0) are
preserved; specifically, use `field.value ?? 'N/A'` (or equivalent) when
rendering the value, ensuring LicenseField.value (which can be number) is
displayed correctly while still showing 'N/A' only for null/undefined.
In `@admin-ui/app/utils/validation/commitMessage.ts`:
- Around line 6-8: The JSDoc for the commit/action message validator in
commitMessage.ts claims the over-limit format is "X characters over limit
(maximum 512)" but the function returns `${excess}
${t('placeholders.charMoreThan512')}`; update the code so the JSDoc matches
actual output or vice versa: either change the JSDoc to state that the message
uses the translation key placeholders.charMoreThan512 prefixed by the excess
count, or replace the translation usage in the function with a
hardcoded/templated string matching "X characters over limit (maximum 512)".
Locate the validator function in commitMessage.ts (the function that computes
`excess` and returns `${excess} ${t('placeholders.charMoreThan512')}`) and make
the documentation and implementation consistent.
- Around line 10-19: The under-minimum validation string in
getCommitMessageValidationError is hardcoded in English while the over-maximum
branch uses t(), causing inconsistent i18n; update the under-minimum branch to
use the translation function (t) with an appropriate key (e.g., a new key for "X
characters required (minimum Y)") and pass remaining and MIN_LENGTH as
interpolation values, or replace the over-maximum branch with a matching
hardcoded string if you intentionally avoid i18n; ensure you reference
MIN_LENGTH, MAX_LENGTH, remaining/excess and use t(...) consistently for both
branches.
In
`@admin-ui/plugins/admin/components/Health/components/ServiceStatusCard.style.ts`:
- Around line 19-59: The current layout in ServiceStatusCard.style uses absolute
positioning for textContainer (right: '80px') and statusBadge, which can cause
badge/text overlap on narrow screens or long strings; update the styles so the
badge participates in the normal layout (e.g., convert the root/content or
textContainer to a flex or grid container and remove the hardcoded right: '80px'
on textContainer and absolute positioning from statusBadge), position the
statusBadge via flex/grid alignment (e.g., alignSelf/end or grid column) and use
gap or margin for spacing; adjust serviceName/serviceMessage to allow wrapping
(auto height) and add a responsive breakpoint or min-width test to ensure no
overlap at the smallest supported width, then run the UI at narrow widths to
verify the badge no longer collides with text.
In `@admin-ui/plugins/admin/components/Health/HealthPage.style.ts`:
- Around line 3-33: Replace the hard-coded borderRadius value in the healthCard
style with the centralized token: update the healthCard object inside useStyles
to use BORDER_RADIUS.DEFAULT instead of 16 and add an import for BORDER_RADIUS
from '@/constants'; target the useStyles function and the healthCard key when
making the change so the Health card uses the centralized sizing token.
In `@admin-ui/plugins/admin/components/Health/HealthPage.tsx`:
- Around line 81-97: Move the inline icon color styles into the stylesheet by
adding new class(es) in HealthPage.style.ts (e.g., errorIcon and infoIcon) and
apply them to the <i> elements in the HealthPage component instead of the inline
style; update the JSX where isError and where !isLoading && !isError &&
services.length === 0 render the icons (currently using className="fa
fa-exclamation-triangle" and className="fa fa-info-circle") to also include the
new classes (e.g., className={`fa fa-exclamation-triangle
${classes.errorIcon}`}), keeping classes.messageBlock unchanged.
- Around line 17-24: HealthPage currently reads theme via
useContext(ThemeContext) and manual null checks; replace that with the
useTheme() hook from 'Context/theme/themeContext' to get a non-nullable theme
API. In the HealthPage component, call useTheme() instead of
useContext(ThemeContext), derive currentTheme from the returned value (falling
back to DEFAULT_THEME if needed), compute isDark by comparing to THEME_DARK, and
keep using getThemeColor(currentTheme) in the useMemo; update any references to
ThemeContext to use the hook (useTheme) and remove manual null-safety code.
admin-ui/plugins/admin/components/Health/components/ServiceStatusCard.style.ts
Outdated
Show resolved
Hide resolved
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.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@admin-ui/app/components/GluuSpinner/GluuSpinner.tsx`:
- Around line 20-21: In the GluuSpinner component's render (the JSX return that
renders <output className={classes.spinner} ... />), remove the explicit
role="status" attribute because the <output> element already implies that role;
keep aria-label and aria-live as they are to preserve accessibility and satisfy
the a11y lint rule.
In `@admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx`:
- Around line 141-148: The overlay currently uses a non-semantic div with
role="button" which breaks Biome's useSemanticElements rule; replace that div
with a proper <button type="button"> element (keeping classes.overlay for
styling) and preserve the handlers and attributes: onClick={closeModal},
onKeyDown={handleOverlayKeyDown}, tabIndex={0} (can be removed when using
button), and aria-label={t('actions.close')}; ensure the element remains
self-closing or empty as before and adjust any CSS if it relied on a div
(GluuCommitDialog.tsx — classes.overlay, closeModal, handleOverlayKeyDown,
t('actions.close')).
In `@admin-ui/app/routes/License/LicenseDetailsPage.tsx`:
- Around line 92-99: The customerName value may render "undefined" when one of
item.customerFirstName or item.customerLastName is missing; update the value
expression in LicenseDetailsPage (the object with key 'customerName') to build
the display by collecting item.customerFirstName and item.customerLastName,
filtering out falsy values, joining with a space, and using '-' as a fallback
(while preserving the existing loading ? PLACEHOLDER branch); reference the
properties item.customerFirstName, item.customerLastName and the PLACEHOLDER
constant when making the change.
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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx`:
- Around line 98-103: The handleAccept callback currently delegates closing to
onAccept and can leave the dialog open; update handleAccept to set the formik
field as now, call onAccept(userMessage), and then directly close the dialog and
run cleanup by invoking onCloseModal() (or the component's close handler) from
inside handleAccept so closing and webhook cleanup don't rely on caller
implementation; reference the handleAccept function, formik.setFieldValue,
onAccept, userMessage, and onCloseModal to locate where to add the direct
close/cleanup call.
In `@admin-ui/app/routes/Apps/Gluu/styles/GluuCommitDialog.style.ts`:
- Around line 22-38: The modalContainer style currently uses height: `min(...,
90vh)` with overflow: 'visible', which allows content to spill on short
viewports; update the modalContainer in GluuCommitDialog.style.ts to enable
vertical scrolling by replacing or augmenting overflow with overflowY: 'auto'
(and optionally overflowX: 'hidden') so the modal respects maxHeight and shows a
scrollbar instead of letting textarea/actions escape; keep existing maxHeight
and other layout rules intact when applying this change.
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.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@admin-ui/app/constants/status.ts`:
- Around line 20-25: STATUS_LABEL_KEYS currently maps the ServiceStatusValue
"unknown" to the same i18n key as "down" ('messages.status_inactive'), which
conflates distinct meanings; change the mapping for "unknown" in
STATUS_LABEL_KEYS to a distinct key (e.g., 'messages.status_unknown') and add
the corresponding i18n entry so the UI can render a different label for the
unknown state.
- Around line 27-32: The STATUS_COLORS constant should be made readonly like the
other constants to get narrower types; update the declaration of STATUS_COLORS
(the Record<ServiceStatusValue, string> object) to append "as const" so its
values are inferred as literal/readonly and consistent with STATUS_LABEL_KEYS,
STATUS_BADGE_COLOR, and STATUS_DETAILS — keep the same keys (up, down, degraded,
unknown) and use the existing customColors references
(customColors.statusActive, customColors.statusInactive, customColors.orange).
In `@admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx`:
- Around line 87-91: The conditional inside the useMemo that computes titleText
uses a redundant check `!label || label === ''`; simplify it to just `!label` by
updating the titleText useMemo (referencing titleText, useMemo, isLicenseLabel,
label, and t) so the second branch reads `if (!label) return
t('messages.action_commit_question_title')` and remove the extra `label === ''`
check.
- Around line 172-190: The textarea with id/name USER_MESSAGE (value
userMessage, onChange handleInputChange, class classes.textarea) lacks ARIA
attributes to expose validation state; add aria-invalid={!!errorMessageText} and
set aria-describedby to the id of the error element (e.g.,
`${USER_MESSAGE}-error`) when errorMessageText is present, and give the GluuText
error element that id (or remove aria-describedby when no error) so assistive
tech can announce the validation message; ensure the error element (GluuText)
retains classes.errorMessage and customColors.statusInactive styling.
- Around line 149-156: Add focus management to the modal by attaching a ref to
the element with className={classes.modalContainer} and, when the dialog opens,
programmatically move focus into the dialog (preferably to the first focusable
element or the dialog container) and trap focus while open; update the existing
handleModalKeyDown to handle Tab/Shift+Tab to cycle focus within the dialog (or
integrate a small focus-trap utility inside the component) and ensure focus is
returned to the trigger on close; reference the modal container ref
(classes.modalContainer), the key handler (handleModalKeyDown) and the existing
aria-labelledby="commit-dialog-title" when locating where to place the
focus-management logic.
- Around line 211-220: The PropTypes for GluuCommitDialog are missing
validations for optional props defined in the GluuCommitDialogProps interface;
update GluuCommitDialog.propTypes to include operations (arrayOf(object)),
alertMessage (string), alertSeverity
(oneOf('error','warning','info','success')), and inputType
(oneOf('text','textarea','email','password','number','tel','url','search')) so
the runtime prop checks match the TypeScript interface and the existing required
entries (handler, onAccept, modal) remain unchanged.
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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@admin-ui/app/constants/status.ts`:
- Around line 5-16: STATUS_MAP currently lists many case variants but can still
miss mixed-case statuses (e.g., "Up"/"Down"); instead normalize the incoming
status string before lookup (e.g., call .toLowerCase() or a locale-safe
normalization) where STATUS_MAP is used, or replace the map keys with lowercased
keys and ensure consumers call lookup with normalized keys; update any functions
that reference STATUS_MAP (lookup sites) to perform the normalization so mapping
always succeeds regardless of input case while keeping the typed
ServiceStatusValue output.
In `@admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx`:
- Around line 27-36: The GluuCommitDialog component no longer uses the props
operations, alertMessage, alertSeverity, and inputType—remove these four
properties from the GluuCommitDialogProps TypeScript interface (in
admin-ui/app/routes/Apps/Gluu/types/GluuCommitDialog.ts) and delete them from
the PropTypes or prop type declaration in the GluuCommitDialog component file
(GluuCommitDialog.tsx), ensuring the destructured props list for the
GluuCommitDialog functional component matches the updated interface (handler,
modal, onAccept, formik, label, placeholderLabel, feature, isLicenseLabel) and
run type checks to confirm no callers rely on the removed props.
|
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin-ui/app/routes/Dashboards/constants.ts (1)
15-41: 🧹 Nitpick | 🔵 TrivialConsider consolidating status display/color maps with centralized constants.
STATUS_DISPLAY_MAPandSTATUS_COLOR_MAPoverlap with the centralizedSTATUS_LABEL_KEYSandSTATUS_BADGE_COLORin@/constants/status.ts. While these serve slightly different purposes (raw display strings vs i18n keys, different color naming), consolidating or clearly documenting the distinction would improve maintainability.If both are needed, consider adding a comment explaining why the dashboard requires separate mappings.
🤖 Fix all issues with AI agents
In `@admin-ui/plugins/admin/components/Health/hooks/useHealthStatus.ts`:
- Line 41: HEALTH_PAGE_EXCLUDED_SERVICES is currently declared inside
useHealthStatus.ts; move this constant to the shared health constants module (or
the local ../constants) alongside STATUS_MAP and STATUS_DETAILS, export it from
that module, then update useHealthStatus.ts to import
HEALTH_PAGE_EXCLUDED_SERVICES instead of declaring it locally and remove the
local declaration; ensure the exported name and type (readonly tuple / as const)
match the original so usages in health-related components remain type-safe.
admin-ui/plugins/admin/components/Health/hooks/useHealthStatus.ts
Outdated
Show resolved
Hide resolved
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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@admin-ui/plugins/admin/components/Health/hooks/useHealthStatus.ts`:
- Around line 13-16: normalizeStatus currently lowercases apiStatus but doesn't
trim stray whitespace, so values like " up " won't match STATUS_MAP and will
fall back to DEFAULT_STATUS; update the normalizeStatus function to first coerce
apiStatus to a string (e.g., default to empty string if undefined/null), then
call .trim() before .toLowerCase() so the lookup against STATUS_MAP (and
fallback to DEFAULT_STATUS) works for inputs with surrounding whitespace.
- Around line 30-41: The transformServiceStatus function sets lastChecked to new
Date() for each service independently; capture a single timestamp once per call
(e.g., const checkedAt = new Date()) before mapping and use that checkedAt for
every ServiceHealth.lastChecked so all services from the same response share the
same timestamp; update transformServiceStatus and keep calls to normalizeStatus
and ServiceStatusResponse casting unchanged.
|



feat(admin-ui): revamp Health, License, and Commit components (#2584)
Summary
This PR revamps the Health, License, and Commit components in the Admin UI to align with the updated design guidelines as part of the broader Admin UI redesign initiative.
Changes Included
Result
Parent Initiative
This work is part of the following parent feature:
feat(admin-ui): Revamp the existing design of the admin-ui according to the new design🔗 Ticket
Closes: #2584
Summary by CodeRabbit
New Features
Improvements
Quality of Life
Localization