feat(admin-ui): revamp Auth Server Keys and Logging modules as per Figma#2708
Merged
feat(admin-ui): revamp Auth Server Keys and Logging modules as per Figma#2708
Conversation
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
…ex into admin-ui-issue-2632
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
…gma (#2641) Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughLarge refactor removing many legacy UI components, converting class components to hooks, and introducing revamped Auth Server Keys and Logging pages with new hooks, styles, types, and tests; also updates i18n and shared form styles and prunes component barrel exports. Changes
Sequence Diagram(s)sequenceDiagram
participant User as rgba(46,204,113,0.5)
participant LoggingPage as rgba(52,152,219,0.5)
participant useLoggingApi as rgba(155,89,182,0.5)
participant QueryClient as rgba(241,196,15,0.5)
participant CommitDialog as rgba(231,76,60,0.5)
participant Toast as rgba(26,188,156,0.5)
User->>LoggingPage: open page
LoggingPage->>useLoggingApi: fetch logging config
useLoggingApi->>QueryClient: read/cache or call API
QueryClient-->>useLoggingApi: return logging data
useLoggingApi-->>LoggingPage: provide data
User->>LoggingPage: change form & click Apply
LoggingPage->>CommitDialog: open with changedFields
User->>CommitDialog: confirm
CommitDialog->>useLoggingApi: mutateAsync(updatedConfig)
useLoggingApi->>QueryClient: setQueryData(updatedConfig)
useLoggingApi-->>LoggingPage: mutation result
LoggingPage->>Toast: show success/error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
mo-auto
added
comp-admin-ui
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@admin-ui/app/components/Layout/Layout.tsx`:
- Around line 161-163: The toggle logic flips state.sidebarCollapsed via
toggleSidebar (using setState), but the LayoutSidebar prop is being passed a
hardcoded sidebarCollapsed: false so the component never sees the real state;
update the LayoutSidebar JSX to forward the actual state value (e.g.,
sidebarCollapsed={state.sidebarCollapsed}) and ensure any handlers still use
toggleSidebar (and that state is in scope) so the sidebar receives the true
collapse state instead of false.
- Around line 121-143: The screenSize change in the useEffect
(layoutAdjuster/onResize) updates state.screenSize but does not trigger
recalculation of navbar offsets, leaving inline top values stale; fix by
invoking updateNavbarsPositions() whenever screenSize actually changes — either
call updateNavbarsPositions() immediately after the setState branch in
layoutAdjuster (the branch that updates prev.screenSize), or add a short
useEffect that watches state.screenSize and calls updateNavbarsPositions();
reference the functions/variables layoutAdjuster, onResize, state.screenSize,
and updateNavbarsPositions to locate and implement the change.
- Around line 172-174: changeMeta currently only updates local LayoutState
(pageTitle, pageDescription, pageKeywords) but never writes these values to
document.title or meta tags, so metadata changes are lost; add a side-effect in
Layout (e.g., a useEffect in the Layout component that watches state.pageTitle,
state.pageDescription, and state.pageKeywords) to: set document.title from
pageTitle (or call SetTitle if it must remain the single writer for UI), and
create/update <meta name="description"> and <meta name="keywords"> elements with
pageDescription and pageKeywords respectively (creating them if absent) so the
head is kept in sync whenever changeMeta is called.
In `@admin-ui/app/components/OuterClick/OuterClick.tsx`:
- Around line 15-18: The code relies on the non-standard Event.path property
(type EventWithPath / DocumentClickEvent) when checking for the nav-toggle,
which fails in non-Chrome browsers; update the click handler in the OuterClick
component to call evt.composedPath() (with a lightweight fallback to (evt as
any).path if you want legacy support), remove/stop using the custom
EventWithPath.path type, and change the nav-toggle exclusion logic to check the
composedPath() array for an element with id 'nav-toggle' (e.g.,
composedPath().some((node: any) => node?.id === 'nav-toggle')) so the
outside-click detection works cross-browser.
In `@admin-ui/app/components/Sidebar/Sidebar.tsx`:
- Around line 61-65: OuterClick is being used as a no-op because active={false}
and no onClickOutside handler are passed; either remove the OuterClick wrapper
around the div if outside-click collapse is no longer desired, or re-enable
outside-click behavior by passing a truthy active prop and an onClickOutside
handler that collapses the sidebar (e.g., call the existing collapse/close
function used by Sidebar or update its open state). Locate the OuterClick usage
in Sidebar.tsx and either delete the OuterClick wrapper or change it to
<OuterClick active={true} onClickOutside={yourCollapseHandler}> (using the
Sidebar's collapse/close function or state updater).
- Around line 18-57: The effect unconditionally constructs and runs
SidebarEntryAnimate/SlimSidebarAnimate/SlimMenuAnimate and the class list always
includes 'sidebar--animations-enabled', ignoring
animationsDisabled/pageConfig.animationsDisabled; update the useEffect in
Sidebar.tsx to check animationsDisabled || pageConfig.animationsDisabled and if
true skip creating/assigning/executing the animation controllers (avoid calling
new SidebarEntryAnimate(), .assignParentElement, and
entryAnimate.executeAnimation), only create them when animations are enabled and
ensure the cleanup only tries to destroy controllers that were created, and
update the sidebarClass construction so 'sidebar--animations-enabled' is added
only when animations are not disabled.
In
`@admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.test.tsx`:
- Around line 100-104: The test currently asserts the hardcoded string "Unnamed
Key" which can drift from translations; update the test to read the expected
label from the app's i18n source used by the component (e.g., call the same
translation helper or i18n.t with the same key the JwkItem component uses)
instead of hardcoding the literal, so in JwkItem.test.tsx replace the direct
"Unnamed Key" expectation with the translated value derived from the same key
(use mockItem, JwkItem, Wrapper, and mockClasses to locate the test and import
the translation helper).
In
`@admin-ui/plugins/auth-server/components/Configuration/Keys/styles/KeysPage.style.ts`:
- Around line 104-108: fieldsGrid currently forces two columns
(gridTemplateColumns: 'repeat(2, 1fr)') which breaks responsive layout on narrow
screens; update the fieldsGrid style to add a mobile breakpoint that overrides
gridTemplateColumns to a single column (e.g., '1fr') at small widths (use your
existing breakpoint utility or a media query) while keeping the same gap
(SPACING.CARD_CONTENT_GAP * 2) so long key values wrap instead of squeezing or
overflowing.
In
`@admin-ui/plugins/auth-server/components/Configuration/Logging/styles/LoggingPage.style.ts`:
- Around line 61-66: The fieldsGrid style forces two fixed columns causing
overflow on narrow viewports; update the fieldsGrid rule to be responsive by
keeping the two-column layout by default (gridTemplateColumns: '1fr 1fr') but
add a small-screen override to collapse to a single column (gridTemplateColumns:
'1fr') using your styling system's breakpoint or a media query (e.g.,
theme.breakpoints.down or `@media` (max-width: ...)); modify the fieldsGrid entry
(and keep SPACING.SECTION_GAP / SPACING.CARD_CONTENT_GAP) so the grid switches
to one column on small screens.
- Around line 48-53: The current focus rule for '& select:focus, &
select:active, & .custom-select:focus, & .form-control:focus' removes
outline/boxShadow leaving no visible keyboard focus; restore a clear,
high-contrast focus indicator (e.g., keep the backgroundColor and border but add
a visible outline or accessible boxShadow like `outline: 2px solid
<accentColor>` or `boxShadow: 0 0 0 3px rgba(<accentColor>, 0.25)`) using
existing tokens (formInputBg, inputBorderColor or an accessible accent color) so
keyboard users can see focus on the select/custom-select/form-control elements.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 81788b50-0d42-4a5f-9b2e-ae13c55f4ca5
📒 Files selected for processing (91)
admin-ui/app/components/Avatar/Avatar.d.tsadmin-ui/app/components/Avatar/Avatar.tsxadmin-ui/app/components/Avatar/AvatarAddonBadge.tsxadmin-ui/app/components/Avatar/AvatarAddonIcon.tsxadmin-ui/app/components/Avatar/AvatarFont.tsxadmin-ui/app/components/Avatar/AvatarImage.tsxadmin-ui/app/components/Avatar/index.tsadmin-ui/app/components/ExtendedDropdown/ExtendedDropdown.tsxadmin-ui/app/components/ExtendedDropdown/index.tsadmin-ui/app/components/Layout/Layout.tsxadmin-ui/app/components/Layout/types.tsadmin-ui/app/components/Nav/index.tsadmin-ui/app/components/Nav/nav.tsxadmin-ui/app/components/Navbar/index.tsadmin-ui/app/components/Navbar/navbar.tsxadmin-ui/app/components/NestedDropdown/NestedDropdown.tsxadmin-ui/app/components/NestedDropdown/context.tsxadmin-ui/app/components/NestedDropdown/index.tsadmin-ui/app/components/OuterClick/OuterClick.tsxadmin-ui/app/components/Progress/Progress.scssadmin-ui/app/components/Progress/Progress.tsxadmin-ui/app/components/Progress/index.tsadmin-ui/app/components/Recharts/CartesianGrid.tsadmin-ui/app/components/Recharts/DefAreaValueColor.tsxadmin-ui/app/components/Recharts/Legend.tsadmin-ui/app/components/Recharts/PieValueLabel.tsxadmin-ui/app/components/Recharts/PolarAngleAxis.tsadmin-ui/app/components/Recharts/PolarGrid.tsadmin-ui/app/components/Recharts/PolarRadiusAxis.tsadmin-ui/app/components/Recharts/Tooltip.tsadmin-ui/app/components/Recharts/ValueLabel.tsxadmin-ui/app/components/Recharts/XAxis.tsadmin-ui/app/components/Recharts/YAxis.tsadmin-ui/app/components/Recharts/ZAxis.tsadmin-ui/app/components/Recharts/config.tsadmin-ui/app/components/Sidebar/Sidebar.tsxadmin-ui/app/components/Sidebar/SidebarClose.tsxadmin-ui/app/components/Sidebar/SidebarContent.tsxadmin-ui/app/components/Sidebar/SidebarHideSlim.tsxadmin-ui/app/components/Sidebar/SidebarMobileFluid.tsxadmin-ui/app/components/Sidebar/SidebarSection.tsxadmin-ui/app/components/Sidebar/SidebarShowSlim.tsxadmin-ui/app/components/Sidebar/index.tsadmin-ui/app/components/Sidebar/types.tsadmin-ui/app/components/SidebarTrigger/SidebarTrigger.tsxadmin-ui/app/components/SidebarTrigger/index.tsadmin-ui/app/components/Theme/ThemeClass.tsxadmin-ui/app/components/Theme/ThemeContext.tsxadmin-ui/app/components/Theme/ThemeSelector.tsxadmin-ui/app/components/Theme/index.tsadmin-ui/app/components/Theme/types.tsadmin-ui/app/components/UncontrolledTabs/UncontrolledTabs.tsxadmin-ui/app/components/UncontrolledTabs/UncontrolledTabsTabContent.tsxadmin-ui/app/components/UncontrolledTabs/context.tsxadmin-ui/app/components/UncontrolledTabs/index.tsadmin-ui/app/components/index.tsxadmin-ui/app/components/recharts.tsadmin-ui/app/layout/components/DefaultSidebar.tsxadmin-ui/app/locales/en/translation.jsonadmin-ui/app/locales/es/translation.jsonadmin-ui/app/locales/fr/translation.jsonadmin-ui/app/locales/pt/translation.jsonadmin-ui/app/routes/Apps/Gluu/GluuNavBar.tsxadmin-ui/app/styles/formStyles.tsadmin-ui/plugins/admin/components/Mapping/styles/MappingPage.style.tsadmin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/utils/validations.tsadmin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.test.tsxadmin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsxadmin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.test.tsxadmin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.tsxadmin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkListPage.test.tsxadmin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkListPage.tsxadmin-ui/plugins/auth-server/components/Configuration/Keys/KeysPage.test.tsxadmin-ui/plugins/auth-server/components/Configuration/Keys/KeysPage.tsxadmin-ui/plugins/auth-server/components/Configuration/Keys/__tests__/fixtures/jwkTestData.tsadmin-ui/plugins/auth-server/components/Configuration/Keys/constants.test.tsadmin-ui/plugins/auth-server/components/Configuration/Keys/hooks/useJwkActions.test.tsadmin-ui/plugins/auth-server/components/Configuration/Keys/hooks/useJwkApi.test.tsadmin-ui/plugins/auth-server/components/Configuration/Keys/styles/KeysPage.style.tsadmin-ui/plugins/auth-server/components/Configuration/Keys/types/jwkTypes.tsadmin-ui/plugins/auth-server/components/Configuration/Logging/LoggingPage.test.tsxadmin-ui/plugins/auth-server/components/Configuration/Logging/LoggingPage.tsxadmin-ui/plugins/auth-server/components/Configuration/Logging/__tests__/fixtures/loggingTestData.tsadmin-ui/plugins/auth-server/components/Configuration/Logging/hooks/index.tsadmin-ui/plugins/auth-server/components/Configuration/Logging/hooks/useLoggingApi.tsadmin-ui/plugins/auth-server/components/Configuration/Logging/styles/LoggingPage.style.tsadmin-ui/plugins/auth-server/components/Configuration/Logging/types.tsadmin-ui/plugins/auth-server/components/Configuration/Logging/utils.tsadmin-ui/plugins/auth-server/components/Configuration/Logging/validations.tsadmin-ui/plugins/auth-server/plugin-metadata.jsadmin-ui/plugins/auth-server/redux/features/types/loggingTypes.ts
💤 Files with no reviewable changes (52)
- admin-ui/app/components/Navbar/index.ts
- admin-ui/app/components/Sidebar/SidebarMobileFluid.tsx
- admin-ui/app/components/Progress/Progress.scss
- admin-ui/app/components/Sidebar/SidebarHideSlim.tsx
- admin-ui/app/components/Recharts/YAxis.ts
- admin-ui/app/components/Recharts/config.ts
- admin-ui/app/components/Layout/types.ts
- admin-ui/app/components/Recharts/ZAxis.ts
- admin-ui/app/components/NestedDropdown/index.ts
- admin-ui/app/components/Progress/Progress.tsx
- admin-ui/app/components/Sidebar/SidebarContent.tsx
- admin-ui/app/components/NestedDropdown/context.tsx
- admin-ui/app/components/Recharts/PolarAngleAxis.ts
- admin-ui/app/components/Sidebar/SidebarClose.tsx
- admin-ui/app/components/Navbar/navbar.tsx
- admin-ui/app/components/ExtendedDropdown/index.ts
- admin-ui/app/components/UncontrolledTabs/index.ts
- admin-ui/app/components/Recharts/PieValueLabel.tsx
- admin-ui/app/components/Avatar/Avatar.d.ts
- admin-ui/app/components/Avatar/AvatarFont.tsx
- admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.test.tsx
- admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx
- admin-ui/app/components/Recharts/XAxis.ts
- admin-ui/app/components/UncontrolledTabs/UncontrolledTabs.tsx
- admin-ui/app/components/Recharts/CartesianGrid.ts
- admin-ui/app/components/Sidebar/SidebarSection.tsx
- admin-ui/app/components/Recharts/Tooltip.ts
- admin-ui/app/components/UncontrolledTabs/UncontrolledTabsTabContent.tsx
- admin-ui/app/components/Avatar/AvatarAddonBadge.tsx
- admin-ui/app/components/Nav/nav.tsx
- admin-ui/app/components/Avatar/AvatarAddonIcon.tsx
- admin-ui/app/components/Nav/index.ts
- admin-ui/app/components/SidebarTrigger/index.ts
- admin-ui/app/components/Avatar/index.ts
- admin-ui/app/components/Sidebar/index.ts
- admin-ui/app/components/Recharts/PolarRadiusAxis.ts
- admin-ui/app/components/Recharts/PolarGrid.ts
- admin-ui/app/components/Progress/index.ts
- admin-ui/app/components/Theme/index.ts
- admin-ui/app/components/Theme/ThemeSelector.tsx
- admin-ui/app/components/ExtendedDropdown/ExtendedDropdown.tsx
- admin-ui/app/components/Avatar/AvatarImage.tsx
- admin-ui/app/components/UncontrolledTabs/context.tsx
- admin-ui/app/components/Recharts/Legend.ts
- admin-ui/app/components/NestedDropdown/NestedDropdown.tsx
- admin-ui/app/components/Recharts/ValueLabel.tsx
- admin-ui/app/components/SidebarTrigger/SidebarTrigger.tsx
- admin-ui/app/components/Avatar/Avatar.tsx
- admin-ui/app/components/Sidebar/SidebarShowSlim.tsx
- admin-ui/plugins/auth-server/redux/features/types/loggingTypes.ts
- admin-ui/app/components/recharts.ts
- admin-ui/app/components/Recharts/DefAreaValueColor.tsx
admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.test.tsx
Show resolved
Hide resolved
admin-ui/plugins/auth-server/components/Configuration/Keys/styles/KeysPage.style.ts
Show resolved
Hide resolved
admin-ui/plugins/auth-server/components/Configuration/Logging/styles/LoggingPage.style.ts
Show resolved
Hide resolved
admin-ui/plugins/auth-server/components/Configuration/Logging/styles/LoggingPage.style.ts
Show resolved
Hide resolved
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
admin-ui/app/components/Sidebar/Sidebar.tsx (1)
60-66: 🛠️ Refactor suggestion | 🟠 Major
OuterClickwrapper is a no-op and should be removed.With
active={false}and noonClickOutsidehandler, theOuterClickcomponent does nothing. This adds unnecessary DOM nesting and component overhead.If outside-click collapse is intentionally removed, delete the wrapper entirely. If it should be re-enabled for mobile collapse behavior, pass
active={true}and an appropriate handler.Remove no-op wrapper
return ( - <OuterClick active={false}> - <div className={sidebarClass} ref={sidebarRef}> - {children} - </div> - </OuterClick> + <div className={sidebarClass} ref={sidebarRef}> + {children} + </div> )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/app/components/Sidebar/Sidebar.tsx` around lines 60 - 66, The OuterClick wrapper around the sidebar is a no-op because it's rendered with active={false} and no handler; remove the unnecessary wrapper by returning the div with className={sidebarClass} and ref={sidebarRef} directly inside the Sidebar component (preserving children), or if outside-click collapse should be supported re-enable by setting OuterClick active={true} and providing an onClickOutside handler that collapses the sidebar (referencing OuterClick, sidebarRef, sidebarClass, and the Sidebar's collapse handler/state).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@admin-ui/app/components/Layout/Layout.tsx`:
- Line 163: toggleSidebar is currently a no-op which breaks sidebar
expand/collapse. Update the Layout component so toggleSidebar actually flips the
sidebarCollapsed state (e.g., call setState or dispatch to toggle
state.sidebarCollapsed) and ensure the function passed into context/provider
replaces the empty useCallback; if the sidebar feature is intentionally removed,
instead remove toggleSidebar from the context and any consumers. Locate the
toggleSidebar definition in Layout.tsx and either implement it to update
state.sidebarCollapsed (via the existing state setter or reducer) or remove its
exports/usages across components.
- Around line 80-90: initialLayoutState currently sets screenSize to an empty
string via a type assertion which is invalid until the effect runs; change the
initializer to compute a valid value by calling getScreenSize() (the same helper
used on mount) so screenSize is one of 'xs'|'sm'|'md'|'lg'|'xl' from the start;
update the initialLayoutState declaration (and ensure getScreenSize is
imported/available in this module) to use getScreenSize() instead of '' as
ScreenSize.
- Around line 176-179: Refactor the component so the custom hook SetTitle is
invoked at the top level of the component (not inside useEffect); remove the
SetTitle call from the useEffect block and call SetTitle(state.pageTitle)
directly within the component body (above any early returns) so the hook runs
unconditionally and follows the Rules of Hooks; keep the existing useEffect for
any other side effects but do not call SetTitle there.
In `@admin-ui/app/components/OuterClick/OuterClick.tsx`:
- Around line 42-60: handleDocumentClick currently depends on the array
reference excludedElements which causes the useCallback to change whenever
callers pass an inline array; either document callers must memoize
excludedElements (e.g., useMemo or store in a ref) or change the component to
track excluded elements by identity instead of array reference: create an
internal ref (e.g., excludedElementsRef) that stores the last element refs,
update it by comparing element.current identities, and use that ref (not the
array) in the useCallback and effect so handleDocumentClick and listener
registration are stable; refer to symbols handleDocumentClick, excludedElements,
useCallback, containerRef, and onClickOutside when making the change.
In `@admin-ui/app/components/Sidebar/Sidebar.tsx`:
- Around line 50-58: The code accesses pageConfig.sidebarSlim directly when
building sidebarClass which can throw if pageConfig is undefined; update usage
to be defensive by either destructuring defaults from pageConfig (e.g., const {
sidebarSlim = false, animationsDisabled = false } = pageConfig || {}) or use
optional chaining (pageConfig?.sidebarSlim) where referenced in the sidebarClass
construction (alongside symbols slim, animationsEnabled, entryAnimationFinished)
so pageConfig is never accessed without a null/undefined guard.
In `@admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.tsx`:
- Around line 110-112: In JwkItem, avoid rendering an empty textarea for missing
x5c by guarding the JSX the same as other optional key fields: only render the
surrounding div and ReadOnlyField for x5c when item.x5c exists and has at least
one entry (e.g., check item.x5c && item.x5c.length or item.x5c?.[0] truthiness)
so the accordion body doesn't show a dead empty field.
- Around line 24-30: The inputs in JwkItem.tsx are uncontrolled because they use
defaultValue; replace defaultValue with a controlled value prop (e.g.,
value={value ?? ''}) on the Input component so updates from the parent re-render
correctly, keep readOnly, and preserve the textarea rows logic (e.g., {...(type
=== 'textarea' ? { rows: getTextareaRows(value) } : {})}); update any usage of
Input in this component to use value instead of defaultValue to make the fields
controlled.
---
Duplicate comments:
In `@admin-ui/app/components/Sidebar/Sidebar.tsx`:
- Around line 60-66: The OuterClick wrapper around the sidebar is a no-op
because it's rendered with active={false} and no handler; remove the unnecessary
wrapper by returning the div with className={sidebarClass} and ref={sidebarRef}
directly inside the Sidebar component (preserving children), or if outside-click
collapse should be supported re-enable by setting OuterClick active={true} and
providing an onClickOutside handler that collapses the sidebar (referencing
OuterClick, sidebarRef, sidebarClass, and the Sidebar's collapse handler/state).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cdbc16f3-8005-41a7-9014-c0697946e28c
📒 Files selected for processing (12)
admin-ui/app/components/EmptyLayout/EmptyLayout.tsxadmin-ui/app/components/Layout/Layout.tsxadmin-ui/app/components/OuterClick/OuterClick.tsxadmin-ui/app/components/Sidebar/Sidebar.tsxadmin-ui/app/locales/en/translation.jsonadmin-ui/app/locales/es/translation.jsonadmin-ui/app/locales/fr/translation.jsonadmin-ui/app/locales/pt/translation.jsonadmin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.test.tsxadmin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.tsxadmin-ui/plugins/auth-server/components/Configuration/Keys/styles/KeysPage.style.tsadmin-ui/plugins/auth-server/components/Configuration/Logging/styles/LoggingPage.style.ts
💤 Files with no reviewable changes (1)
- admin-ui/app/components/EmptyLayout/EmptyLayout.tsx
admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
admin-ui/app/components/Layout/Layout.tsx (1)
96-96:⚠️ Potential issue | 🔴 CriticalMove
SetTitleback to the component body.
SetTitleis a custom hook in this codebase. Invoking it insideuseEffectand behind a conditional breaks the Rules of Hooks, and it also leaves the previous title in place whenpageTitleis cleared.🪝 Minimal fix
const location = useLocation() const [state, setState] = useState<LayoutState>(initialLayoutState) + SetTitle(state.pageTitle ?? '') const containerRef = useRef<HTMLDivElement>(null) const bodyRef = useRef<HTMLElement | null>(null) const documentRef = useRef<HTMLElement | null>(null) @@ useEffect(() => { - if (state.pageTitle != null) { - SetTitle(state.pageTitle) - } - if (typeof document === 'undefined') returnBased on learnings:
SetTitle(imported from 'Utils/SetTitle') is a custom React hook that should be called at the component's top level, not insideuseEffector other hooks, because it internally manages side effects for setting page titles.Also applies to: 178-181
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/app/components/Layout/Layout.tsx` at line 96, The component currently calls the custom hook SetTitle inside a useEffect and behind a conditional which violates the Rules of Hooks and prevents the title from resetting when pageTitle is cleared; move the SetTitle(pageTitle) invocation to the top level of the component body (immediately after imports/state declarations) so it always runs unconditionally, remove any SetTitle calls from the useEffect block (and lines referenced 178-181), and rely on the imported SetTitle hook to handle setting/resetting the document title when pageTitle changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@admin-ui/app/components/Layout/Layout.tsx`:
- Around line 167-171: The setter setElementsVisibility in Layout.tsx currently
only copies navbarHidden and footerHidden into state so sidebarHidden is
ignored; update the pick call in setElementsVisibility (and the other identical
occurrence) to include 'sidebarHidden' so LayoutState.sidebarHidden is
preserved, and ensure the sidebar render path (where the sidebar is
cloned/conditioned) checks state.sidebarHidden before cloning or rendering the
Sidebar component so callers can actually hide the sidebar via
PageConfigContext.
In `@admin-ui/app/components/Sidebar/Sidebar.tsx`:
- Around line 21-47: The effect for Sidebar (the useEffect that depends on
animationsEnabled) leaves entryAnimationFinished true when animations are
initially disabled, so when animationsEnabled flips to true the entry animation
is skipped; modify the effect to setEntryAnimationFinished(false) immediately
before calling entryAnimate.executeAnimation(), and add a local cancelled flag
(e.g., let cancelled = false) that is set true in the returned cleanup to avoid
updating state after unmount/cleanup; ensure you still call
entryAnimate.destroy()/slimAnimate.destroy()/menuAnimate.destroy() in the
cleanup and only call setEntryAnimationFinished(true) inside the
executeAnimation().then callback when not cancelled.
In `@admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.tsx`:
- Around line 110-114: The conditional rendering uses `item.x5c?.length > 0`,
which relies on implicit coercion; change the guard to an explicit check for a
non-empty array (e.g., `Array.isArray(item.x5c) && item.x5c.length > 0`) before
rendering the ReadOnlyField in JwkItem (the block that references item.x5c and
renders ReadOnlyField with label "x5c") so the intent is clear and
undefined/null values are handled explicitly.
---
Duplicate comments:
In `@admin-ui/app/components/Layout/Layout.tsx`:
- Line 96: The component currently calls the custom hook SetTitle inside a
useEffect and behind a conditional which violates the Rules of Hooks and
prevents the title from resetting when pageTitle is cleared; move the
SetTitle(pageTitle) invocation to the top level of the component body
(immediately after imports/state declarations) so it always runs
unconditionally, remove any SetTitle calls from the useEffect block (and lines
referenced 178-181), and rely on the imported SetTitle hook to handle
setting/resetting the document title when pageTitle changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: aa363105-18d1-4c2f-880b-f76d49f0024c
📒 Files selected for processing (4)
admin-ui/app/components/Layout/Layout.tsxadmin-ui/app/components/OuterClick/OuterClick.tsxadmin-ui/app/components/Sidebar/Sidebar.tsxadmin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.tsx
admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@admin-ui/app/components/Layout/Layout.tsx`:
- Around line 207-236: The provider and the cloned LayoutSidebar are forcing
sidebarSlim to false which prevents slim mode from reaching consumers; update
PageConfigContext.Provider's value to pass the actual state.sidebarSlim (instead
of sidebarSlim: false) and stop overriding it when cloning LayoutSidebar (pass
sidebarSlim: state.sidebarSlim or omit the prop so the child reads from
context), ensuring LayoutSidebar and SidebarMenu can react to slim mode; if slim
mode is intentionally removed, remove the sidebarSlim field and related branches
across PageConfigContext, LayoutSidebar, and SidebarMenu instead of silently
disabling them.
In `@admin-ui/app/components/Sidebar/Sidebar.tsx`:
- Around line 11-12: The Sidebar component currently ignores the collapsed prop
declared in SidebarProps; update the component to honor collapsed by adding
"collapsed" to the props destructuring (alongside animationsDisabled,
pageConfig, slim, children) and include logic that adds the "sidebar--collapsed"
modifier class when collapsed is true (the same place where "slim" is used to
toggle a class in the Sidebar render/className generation). Alternatively, if
you intend to remove the prop, delete collapsed from SidebarProps and any
callers, but the quick fix is to read the collapsed prop in Sidebar and apply
the "sidebar--collapsed" class so callers receive the expected behavior.
In `@admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.tsx`:
- Around line 47-69: The accordion header is implemented as a div with custom
keyboard handlers (handleKeyDown) and role/tabIndex; replace that div with a
native <button type="button"> using the existing headerClassName,
onClick={toggle}, aria-expanded={isOpen} and add aria-controls pointing to the
panel id so screen readers can link them; remove role="button", tabIndex, and
onKeyDown/handleKeyDown (or delete handleKeyDown if unused) since the button
provides correct keyboard/focus semantics and keep any existing classes
(classes.accordionHeader / classes.accordionHeaderOpen) and toggle usage
unchanged.
- Line 71: JwkItem currently renders item.name or the generic
t('fields.unnamed_key'), which hides item.kid; change the rendering logic in the
JwkItem component to prefer item.name, then item.kid, then the unnamed fallback
(e.g., use item.name ?? item.kid ?? t('fields.unnamed_key')) so keys that lack a
friendly name still display their stable identifier (consistent with JwkListPage
handling of kid).
- Around line 14-16: getTextareaRows currently computes wrapped rows from the
total string length and then adds newline-based rows again, double-counting
multiline values; change getTextareaRows to split the input into lines and sum
Math.ceil(line.length / CHARS_PER_LINE) for each line (treat empty lines as 1
row), then clamp that total between MIN_TEXTAREA_ROWS and MAX_TEXTAREA_ROWS
(refer to the getTextareaRows function and the CHARS_PER_LINE,
MIN_TEXTAREA_ROWS, MAX_TEXTAREA_ROWS constants).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0066f4a8-200a-4627-a21e-e369f801bca4
📒 Files selected for processing (3)
admin-ui/app/components/Layout/Layout.tsxadmin-ui/app/components/Sidebar/Sidebar.tsxadmin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.tsx
admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.tsx
Show resolved
Hide resolved
admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.tsx
Show resolved
Hide resolved
admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.tsx
Show resolved
Hide resolved
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.tsx`:
- Around line 153-172: The component currently guards rendering with "item.x &&
item.y" which hides valid JWKs that only provide x; update the condition in
JwkItem (the JSX that renders ReadOnlyField for "x" and "y" inside
classes.fieldsGrid / classes.fieldItem) so that the "x" ReadOnlyField is
rendered when item.x exists regardless of item.y, while still rendering the "y"
ReadOnlyField only when item.y exists; keep the surrounding layout and use
emptyPlaceholder as before.
In
`@admin-ui/plugins/auth-server/components/Configuration/Keys/styles/KeysPage.style.ts`:
- Around line 60-69: The accordionHeader style currently strips keyboard focus
by setting 'outline: none' on ':focus' and ':focus-visible'; restore an
accessible focus indicator by removing those 'outline: none' rules from the
accordionHeader rules and instead add a visible focus style (for example a clear
outline or box-shadow and/or change background/border) in the '&:focus' and
'&:focus-visible' selectors so keyboard users can see which accordion header
(accordionHeader) is focused.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cad61e6e-9855-4dbe-a292-f1643acc72e2
📒 Files selected for processing (7)
admin-ui/app/locales/en/translation.jsonadmin-ui/app/locales/es/translation.jsonadmin-ui/app/locales/fr/translation.jsonadmin-ui/app/locales/pt/translation.jsonadmin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.tsxadmin-ui/plugins/auth-server/components/Configuration/Keys/styles/KeysPage.style.tsadmin-ui/plugins/auth-server/components/Configuration/Keys/types/jwkTypes.ts
admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/auth-server/components/Configuration/Keys/styles/KeysPage.style.ts
Show resolved
Hide resolved
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
|
duttarnab
approved these changes
Mar 19, 2026
moabu
approved these changes
Mar 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat(admin-ui): revamp Auth Server Keys and Logging modules as per Figma (#2641)
Summary
This PR revamps the Auth Server → Keys and Auth Server → Logging modules in the Admin UI to align with the latest Figma designs as part of the ongoing 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: #2641
Summary by CodeRabbit
New Features
Localization
Bug Fixes & Improvements
Chores
Tests