feat(admin-ui): revamp Cedarling Policy, Role & Mapping as per Figma#2645
Merged
feat(admin-ui): revamp Cedarling Policy, Role & Mapping as per Figma#2645
Conversation
Contributor
📝 WalkthroughWalkthroughAdds Cedarling-specific spacing and color tokens, extends themeConfig, introduces URL validators and ID regexes, migrates text to GluuText, refactors GluuFormFooter, rewrites Cedarling and Mapping UIs with themed styles and new role/permission components, and adds comprehensive Redux types and typed hooks. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant CedarlingPage as CedarlingConfigPage
participant Validator as "URL Validator"
participant Backend as BackendAPI
participant Toast as Toast/Alert
User->>CedarlingPage: Click "Apply" (URL + retrievalPoint)
CedarlingPage->>Validator: validate(trimmed URL) if retrievalPoint == "remote"
alt validation fails
Validator-->>CedarlingPage: invalid
CedarlingPage->>Toast: show validation error
else validation passes / retrievalPoint == "default"
Validator-->>CedarlingPage: valid
CedarlingPage->>Backend: submit trimmed payload
Backend-->>CedarlingPage: success / error
CedarlingPage->>Toast: show success or 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 |
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: 5
🤖 Fix all issues with AI agents
In `@admin-ui/app/routes/Apps/Gluu/GluuFormFooter.tsx`:
- Around line 54-67: The computed hasRightGroup logic in buttonStates is
incorrect (it uses !!showCancel && !showBack) causing the Cancel button to
vanish when showBack and showCancel are true but showApply is false; update the
hasRightGroup expression inside the useMemo for buttonStates (referencing
buttonStates, hasRightGroup, showBack, showCancel, showApply) so it becomes true
when there are three buttons OR when Cancel exists and Apply does not (e.g.,
hasRightGroup = hasThreeButtons || (showCancel && !showApply)).
In `@admin-ui/app/utils/validation/url.ts`:
- Around line 1-18: Both validators (isValidUrl and isValidUrlAnyProtocol) call
url.trim() only for the emptiness check but still pass the original string to
new URL, causing valid URLs with surrounding whitespace to be rejected; fix by
assigning const trimmed = url.trim(), use trimmed for the emptiness check and
pass trimmed into new URL in both isValidUrl and isValidUrlAnyProtocol (keeping
the existing protocol check in isValidUrl intact).
In `@admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx`:
- Around line 94-128: The form currently allows a sync to run even when
auiPolicyStoreUrl is empty or invalid because sync is only partly validated;
update handleSubmit to fully guard the sync/audit flow by checking
cedarlingPolicyStoreRetrievalPoint === 'remote' AND that
auiPolicyStoreUrl.trim() is non-empty and isValidUrl(auiPolicyStoreUrl) before
performing any sync or audit-log actions (same validation used in the urlError
and toast branches). Ensure any early returns occur before calling the sync
logic (the code around lines 155-165) and surface the same user-facing error
toasts when the URL is missing/invalid so sync is never attempted without a
valid remote URL.
In `@admin-ui/plugins/admin/components/Mapping/MappingItem.tsx`:
- Around line 83-96: The clickable header Box (classes.roleCardHeader) lost
keyboard and ARIA support; update the element that uses handleToggle and
ExpandMore so it is focusable and accessible by adding role="button",
tabIndex={0}, aria-expanded={isExpanded}, and aria-controls referencing the
related content panel id (generate a stable id from candidate?.role or a mapping
id), and implement an onKeyDown handler that calls handleToggle when Enter or
Space is pressed; ensure the accessible attributes and onKeyDown are tied to the
same element that currently has onClick so keyboard and screen-reader users can
toggle the section.
In `@admin-ui/plugins/admin/components/Mapping/styles/MappingPage.style.ts`:
- Around line 36-44: The pageDescription style in MappingPage.style.ts currently
sets fontSize to fontSizes['2xl'] (24px) but lineHeight to '16.729px', which
risks clipping; update the lineHeight in the pageDescription object to be at
least the font size (e.g., '24px' or a relative value like '1.25' or '1.5') so
text won’t overlap across locales — locate the pageDescription style object and
replace the '16.729px' value with an appropriate ≥24px/≥1.0 relative
line-height.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@admin-ui/app/routes/Apps/Gluu/GluuFormFooter.tsx`:
- Around line 113-126: Multiple GluuButton instances repeat styling props
(useOpacityOnHover, hoverOpacity, style={commonButtonStyle}); create a shared
props object (e.g., const sharedButtonProps = { useOpacityOnHover: true,
hoverOpacity: 0.85, style: commonButtonStyle }) and spread it into each
GluuButton to remove duplication; update the GluuButton uses around
handleBackClick/backLabel/disableBack and the other button handlers (the other
GluuButton blocks referenced) to include ...sharedButtonProps and keep unique
props (type, onClick, disabled, title, backgroundColor, textColor, borderColor,
children) unchanged.
In `@admin-ui/plugins/admin/components/Mapping/MappingItem.tsx`:
- Around line 16-31: PermissionCheckbox renders a visual-only checkmark so
screen readers can't tell assignment; update PermissionCheckbox to expose state
by adding semantic accessibility attributes: set role="checkbox" and
aria-checked={isChecked} on the outer container Box (the element created in
PermissionCheckbox), add tabIndex={0} if this element should be focusable,
ensure the decorative Check icon has aria-hidden="true", and provide an
accessible name via aria-label or aria-labelledby (e.g., combine the permission
string with "assigned" / "unassigned") so assistive tech reads "permission name,
assigned" when isChecked is true.
- Around line 66-68: The generated contentId (in the useMemo for contentId using
candidate?.role) can produce duplicate or invalid HTML IDs; update the logic to
build a truly unique, valid ID by including a per-item unique piece (e.g., the
item index or a unique identifier from the candidate object) and fully
sanitizing characters beyond whitespace (strip/replace any non-alphanumeric,
underscore, or hyphen). Locate the useMemo that defines contentId and change it
to combine candidate?.role with the item index or candidate.id and run a regex
that removes/normalizes all invalid ID characters so aria-controls references
are unique and valid.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx (1)
260-263:⚠️ Potential issue | 🟠 MajorRefresh button is always hidden due to inverted condition.
Line 260-263 hides the refresh button whenever the retrieval point isremote, and it is also hidden when the URL is empty. Given there are onlyremote/default, this makes the button unreachable in all states. The visibility check should hide only when the retrieval point isdefaultor the URL is empty.🛠️ Proposed fix
- const isRefreshButtonHidden = useMemo( - () => cedarlingPolicyStoreRetrievalPoint === 'remote' || auiPolicyStoreUrl.trim() === '', + const isRefreshButtonHidden = useMemo( + () => cedarlingPolicyStoreRetrievalPoint === 'default' || auiPolicyStoreUrl.trim() === '', [cedarlingPolicyStoreRetrievalPoint, auiPolicyStoreUrl], )admin-ui/app/locales/es/translation.json (1)
4-4:⚠️ Potential issue | 🟡 MinorPotential typo: missing space between words.
The value
"AceptarAgregar mapeo de atributos"appears to concatenate "Aceptar" and "Agregar" without a space. This should likely be"Agregar mapeo de atributos"(to match the keyadd_attribute_mapping) or if both actions are intended, add a space or separator.Proposed fix
- "add_attribute_mapping": "AceptarAgregar mapeo de atributos", + "add_attribute_mapping": "Agregar mapeo de atributos",
🤖 Fix all issues with AI agents
In `@admin-ui/app/utils/regex.ts`:
- Around line 8-9: REGEX_ID_TRIM_HYPHENS currently matches only a single leading
or trailing hyphen; update the pattern used by REGEX_ID_TRIM_HYPHENS to match
one or more hyphens at either end (e.g., change the regex from /^-|-$/g to use
quantifier so it becomes /^-+|-+$/g) so replace calls will remove consecutive
leading/trailing hyphens like "---test---".
In `@admin-ui/plugins/admin/components/Mapping/RolePermissionMappingPage.tsx`:
- Around line 57-63: Replace the hard-coded path string in the Link component
inside RolePermissionMappingPage (where Link currently uses
"/adm/cedarlingconfig") with the canonical route constant from your ROUTES
enum/object (e.g., ROUTES.CEDARLING_CONFIG); update imports to bring in ROUTES
if not already imported, and keep the Link props and classes.infoLink unchanged
so the UI and styling remain the same.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@admin-ui/app/redux/reducers/ReducerRegistry.ts`:
- Around line 32-34: The hasReducer method currently uses the `in` operator
which can match inherited properties; update hasReducer (method name:
hasReducer, property: this._reducers) to check only own properties by using
Object.prototype.hasOwnProperty.call(this._reducers, name) (or
this._reducers.hasOwnProperty(name) guarded via call) so prototype-chain keys
like toString won't return true.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@admin-ui/app/redux/hooks.ts`:
- Around line 1-13: Remove the unused AppStore type from hooks.ts and eliminate
duplicate AppDispatch/exported useAppDispatch definitions elsewhere; keep the
canonical AppDispatch = typeof store.dispatch and useAppDispatch in hooks.ts,
delete or refactor the other AppDispatch and useAppDispatch declarations (the
ones typing AppDispatch as Dispatch<UnknownAction>) and update any modules that
referenced the old duplicates to import AppDispatch/useAppDispatch from the
hooks.ts implementation instead.
In `@admin-ui/app/redux/reducers/ReducerRegistry.ts`:
- Around line 3-4: The local type alias ReducerMap in ReducerRegistry.ts shadows
an exported ReducerMap from redux/types/index.ts which can confuse readers;
rename this local type to InternalReducerMap (or add a clear inline comment) and
update all usages (e.g., the ReducerMap reference where Reducer<unknown,
UnknownAction> and ChangeListener use it) to the new name so it’s clear this is
a looser, dynamic registry type distinct from the stricter exported [K in keyof
RootState]? type.
In `@admin-ui/app/redux/types/index.ts`:
- Around line 554-559: The file defines a strongly-typed ReducerMap keyed by
keyof RootState but ReducerRegistry.ts uses its own looser type Record<string,
Reducer<unknown, UnknownAction>>; update ReducerRegistry to import and use the
exported ReducerMap type from admin-ui/app/redux/types/index.ts (or, if dynamic
plugin keys are intentional, add a brief comment in ReducerRegistry.ts
explaining why a looser Record<string, Reducer<unknown, UnknownAction>> is
required) so the project either consistently leverages the RootState-based
typing (use ReducerMap) or clearly documents the exception.
- Around line 549-552: The AppDispatch, useAppDispatch, and useAppSelector
definitions in this file duplicate the versions in hooks.ts (where AppDispatch
is derived as typeof store.dispatch); remove these three exports from
admin-ui/app/redux/types/index.ts and instead re-export the accurate types/hooks
from admin-ui/app/redux/hooks.ts (or consolidate all typed hooks into one
module), ensuring consumers import AppDispatch, useAppDispatch, and
useAppSelector from the single source that uses typeof store.dispatch to avoid
type drift.
In `@admin-ui/plugins/admin/components/Mapping/RolePermissionMappingPage.tsx`:
- Around line 75-86: The UI shows nothing when canReadMapping is true but
mapping is empty; update the GluuViewWrapper block that currently renders
mapping.map(...) (and the RolePermissionCard) to render an empty-state message
or component when mapping is empty (e.g., mapping.length === 0) after loading.
Use a clear user-facing string like "No role mappings found" (or a reusable
EmptyState/Alert/Typography component) and ensure it displays in place of the
Box list so users get feedback instead of a blank area; keep existing
RolePermissionCard rendering for the non-empty case.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx (1)
255-268:⚠️ Potential issue | 🟠 MajorRefresh action is unreachable due to inverted visibility logic.
Line 255 hides the refresh button whenever the retrieval point is
remote, and Line 241-243 clears the URL fordefault, so the button never renders. If the intent is to allow refresh when a remote URL is set, the condition should be inverted.🛠️ Suggested fix
- const isRefreshButtonHidden = useMemo( - () => cedarlingPolicyStoreRetrievalPoint === 'remote' || auiPolicyStoreUrl.trim() === '', - [cedarlingPolicyStoreRetrievalPoint, auiPolicyStoreUrl], - ) + const isRefreshButtonHidden = useMemo( + () => cedarlingPolicyStoreRetrievalPoint !== 'remote' || auiPolicyStoreUrl.trim() === '', + [cedarlingPolicyStoreRetrievalPoint, auiPolicyStoreUrl], + )
🤖 Fix all issues with AI agents
In `@admin-ui/app/locales/en/translation.json`:
- Around line 1890-1899: The audit event auditSetPolicyStoreAsDefault currently
sends an empty payload ({}); change its payload to include the auiPolicyStoreUrl
(same approach as used by auditSyncRoleToScopesMappings which passes requestData
including auiPolicyStoreUrl) so the audit record contains the Policy Store
URL—locate where auditSetPolicyStoreAsDefault is emitted and replace the {}
payload with an object containing auiPolicyStoreUrl (e.g., { auiPolicyStoreUrl:
requestData.auiPolicyStoreUrl } or equivalent).
In `@admin-ui/app/locales/es/translation.json`:
- Around line 1880-1889: The audit label key "auditSyncRoleToScopesMappings"
uses "función" — replace that value with wording using "rol" or "roles" to match
existing RBAC terminology (e.g., change "Sincronizar función con asignaciones de
ámbitos" to "Sincronizar rol con asignaciones de ámbitos" or "Sincronizar roles
con asignaciones de ámbitos") so it aligns with the UI's "roles" labels; update
the value for the "auditSyncRoleToScopesMappings" JSON entry in translation.json
accordingly.
In `@admin-ui/app/locales/pt/translation.json`:
- Around line 1734-1743: The translation uses inconsistent PT terms—replace
"Predefinido" with "Padrão" and "âmbitos" with "escopos" in the affected keys to
match the rest of the locale: update the values for "useRemotePolicyStore"
(change "Predefinido" → "Padrão"), "policyUrlDisabledWhenDefault" (change
"Predefinido" → "Padrão"), and "auditSyncRoleToScopesMappings" (change "âmbitos"
→ "escopos"); ensure any other occurrences in this JSON block follow the same
replacements so UI terminology is consistent.
In `@admin-ui/plugins/admin/components/Mapping/RolePermissionMappingPage.tsx`:
- Around line 77-79: The empty-state Alert in RolePermissionMappingPage uses
severity="info" but applies classes.errorAlert which misaligns semantics and
visuals; update the component to use a dedicated info/empty-state class (e.g.,
classes.infoAlert or classes.emptyState) instead of classes.errorAlert, and add
the corresponding style rules in the component's styles definition so the
Alert's visuals match severity="info" while preserving errorAlert for real error
states.
admin-ui/plugins/admin/components/Mapping/RolePermissionMappingPage.tsx
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@admin-ui/app/locales/es/translation.json`:
- Line 1886: Replace the English value for the key
"gluuFlexAdminUiPolicyStoreDisplay" with a Spanish translation; update the JSON
entry value from "Gluu Flex Admin UI Policy Store" to a Spanish string such as
"Almacén de políticas de la interfaz de administración de Gluu Flex" while
keeping the key name unchanged and ensuring valid JSON quoting/comma placement.
In `@admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx`:
- Line 120: The validation uses auiPolicyStoreUrl instead of the trimmed
version, causing inconsistent checks; update the URL format validation to use
trimmedUrl (the variable defined earlier) wherever isValidUrl is called so all
empty-checks and format validations operate on the same trimmed value (e.g.,
replace isValidUrl(auiPolicyStoreUrl) with isValidUrl(trimmedUrl) in the
CedarlingConfigPage validation logic).
In `@admin-ui/plugins/admin/components/Mapping/RolePermissionMappingPage.tsx`:
- Around line 69-90: The UI shows both the error Alert and the empty-state Alert
when mapping is empty and isError is true; update the rendering in
RolePermissionMappingPage so the empty-state only appears when there is no error
(i.e., change the condition around mapping.length === 0 to require !isError &&
mapping.length === 0) and ensure the mapping.map branch is only reached when
!isError (so replace the current ternary logic to check !isError before
rendering the info Alert or mapping list).
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/admin/components/Mapping/RolePermissionMappingPage.tsx
Outdated
Show resolved
Hide resolved
faisalsiddique4400
changed the title
duttarnab
requested changes
Feb 5, 2026
Contributor
There was a problem hiding this comment.
- The following button is missing
- The button has following tooltip:
- Please hide the unchecked scopes (i.e. the scopes which the user don't have the permission). We don't want to show too much details to the logged-in users.
-
Can we set Dark mode as the default after login? It looks good. If a user selects Light mode, the Admin UI can use Light mode from the next login onward.
-
The alert font is tiny
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@admin-ui/app/locales/pt/translation.json`:
- Around line 1735-1744: Two locale entries remain in English; update the values
for the keys "agamaLabPolicyDesigner" and "gluuFlexAdminUiPolicyStoreDisplay" in
the PT translation to Portuguese—replace "Agama Lab's Policy Designer" with
something like "Designer de Políticas do Agama Lab" and replace "Gluu Flex Admin
UI Policy Store" with "Armazenamento de Políticas da UI Administrativa Gluu
Flex" so they match the rest of the file.
In `@admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx`:
- Around line 246-251: The handler currently clears the saved remote Policy
Store URL causing the URL to be lost when switching to 'default'; update
handleRadioChange to stop calling setAuiPolicyStoreUrl('') when value ===
'default' so the existing URL is preserved, and make the same change in the
analogous handler around lines 285-289 (the other radio change handler) to
ensure the refresh action remains available and the URL is not persisted as
empty on save; leave setCedarlingPolicyStoreRetrievalPoint behavior intact.
In `@admin-ui/plugins/admin/components/Mapping/RolePermissionCard.tsx`:
- Around line 130-140: The empty-state check in the RolePermissionCard component
is using allPermissions.length but the rendered grid uses permissionCheckboxes
(derived from sortedPermissions), causing inconsistent behavior; update the
condition to check the same source used for rendering (e.g.,
sortedPermissions.length or rolePermissions.size) so the "no permissions"
message appears only when there are no permissionCheckboxes to display; locate
RolePermissionCard and replace the allPermissions.length === 0 check with
sortedPermissions.length === 0 (or rolePermissions.size === 0 if you want to
reflect assigned permissions) to make the empty-state logic consistent with
permissionCheckboxes.
- Around line 28-49: The PermissionCheckbox component renders a focusable,
display-only checkbox (role="checkbox" with tabIndex={0}) but lacks an explicit
readonly state; update PermissionCheckbox to add aria-readonly="true" to the
outer Box (the element with role="checkbox") so assistive tech knows it cannot
be toggled while keeping existing aria-checked and tabIndex behavior.
- Around line 68-70: The current filter in sortedPermissions
(allPermissions.filter((p) => rolePermissions.has(p)).sort()) produces a list of
only assigned permissions but PermissionCheckbox still defines ARIA_UNASSIGNED
and computes aria-label using isChecked, making ARIA_UNASSIGNED dead code;
either: (A) if this UI is read-only, remove ARIA_UNASSIGNED from
PermissionCheckbox and simplify the aria-label to always indicate assigned (use
ARIA_ASSIGNED only) and remove any unused isChecked logic, or (B) if editing is
intended, change sortedPermissions to include allPermissions (e.g.,
allPermissions.sort()) and wire up a toggle handler in RolePermissionCard that
passes isChecked (rolePermissions.has(permission)) and an onChange to
PermissionCheckbox to add/remove permissions; update prop types accordingly.
Ensure you update references to ARIA_ASSIGNED/ARIA_UNASSIGNED and
PermissionCheckbox props to match the chosen approach.
In `@admin-ui/plugins/admin/components/Mapping/RolePermissionMappingPage.tsx`:
- Around line 83-90: The key for RolePermissionCard inside the mapping.map
currently uses candidate?.role || idx which can collide or fall back to index
for falsy roles; update the key generation used in the RolePermissionCard render
(mapping.map -> RolePermissionCard) to produce a deterministic unique key such
as combining role and index (e.g. `${candidate?.role}-${idx}`) or, if available,
use a unique identifier field on candidate (e.g. candidate.id) to ensure stable,
unique keys.
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/admin/components/Mapping/RolePermissionCard.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/admin/components/Mapping/RolePermissionMappingPage.tsx
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@admin-ui/app/locales/pt/translation.json`:
- Around line 816-822: Remove the redundant ICU-style key "permissions_count"
and keep the suffix-based plural keys used by i18next (permissions_count_one and
permissions_count_other); locate the block containing "permissions_count",
"permissions_count_one", and "permissions_count_other" in the pt translation
file and delete the "permissions_count" entry so only the _one and _other keys
remain for correct plural resolution.
In `@admin-ui/app/routes/Dashboards/DashboardPage.tsx`:
- Around line 91-92: You are calling useAppSelector twice against the same slice
(authReducer) which creates two subscriptions; combine them into a single
selector call so DashboardPage reads { isUserInfoFetched, hasSession,
permissions } from state.authReducer in one useAppSelector invocation (keep the
selector shape and names to avoid downstream changes). Update any references to
the separate constants accordingly so the component uses the consolidated object
properties.
- Around line 3-4: Replace the untyped useDispatch import and usage with the
typed hook exported from your hooks module: remove the import of useDispatch
from 'react-redux' and import useAppDispatch from '@/redux/hooks' alongside
useAppSelector, then update the component's dispatch initialization from calling
useDispatch() (symbol: useDispatch) to calling useAppDispatch() (symbol:
useAppDispatch) so the DashboardPage component uses the typed dispatch
consistently with useAppSelector.
In `@admin-ui/app/routes/License/LicenseDetailsPage.style.ts`:
- Around line 63-68: The styles for resetButton and refreshIcon use hardcoded
values; replace gap: 8 with the design token (e.g., SPACING.CARD_GAP) and
fontSize: 16 with the appropriate font size token (e.g., fontSizes.md) to match
the rest of the file; update the resetButton and refreshIcon entries in
LicenseDetailsPage.style.ts to reference those constants (importing them if not
already present).
In `@admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.style.ts`:
- Around line 7-16: The CedarlingConfigThemeColors interface is not exported, so
consumers cannot type the themeColors parameter passed to useStyles; export the
interface by adding an export modifier to CedarlingConfigThemeColors (so callers
can import and use that type when constructing themeColors for useStyles) and
ensure any related exports (if you have a default export pattern) remain
consistent with the module's API.
- Around line 203-205: The hover rule currently builds a fragile hex-alpha
string by appending '14' to customColors.logo (in the '&:hover'
backgroundColor), which breaks if customColors.logo is not a hex string; import
MUI's alpha utility (from '@mui/material/styles') and replace the concat with
alpha(customColors.logo, 0.08) (or equivalent 8% opacity) to produce a
format-safe translucent color for the '&:hover' backgroundColor.
- Around line 65-66: Extract the hardcoded sizing/spacing literals in
CedarlingConfigPage.style.ts into named constants and replace the magic numbers:
create ICON_SIZE.MD for the 24x24 icon size (replace width and height), add
CEDARLING_CONFIG_SPACING.ALERT_TITLE_MB for the `8` margin-bottom, define an
INPUT_PADDING constant for `'14px 21px'`, add TOOLTIP_MAX_WIDTH = 320 for the
tooltip max-width, use CEDARLING_CONFIG_SPACING.RADIO_GROUP_GAP for `'25px'`
radio group gap, and a BUTTON_OFFSET (or similar) for the `4` button offset;
declare these constants near the top of the file (or in the existing spacing
constants module if present) and update the related style rules to reference the
new constant names (target the style objects/selectors where width/height,
marginBottom, padding, maxWidth, gap, and offset are currently set).
Comment on lines
+7
to
+16
| interface CedarlingConfigThemeColors { | ||
| cardBg: string | ||
| navbarBorder: string | ||
| text: string | ||
| alertText: string | ||
| infoBg: string | ||
| infoBorder: string | ||
| inputBg: string | ||
| placeholderText: string | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Export the CedarlingConfigThemeColors interface for consumers.
The interface is required by consumers to construct the themeColors parameter when calling useStyles(). Without exporting it, callers would need to duplicate the type definition or use inline types.
♻️ Proposed fix
-interface CedarlingConfigThemeColors {
+export interface CedarlingConfigThemeColors {
cardBg: string
navbarBorder: string
text: string📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| interface CedarlingConfigThemeColors { | |
| cardBg: string | |
| navbarBorder: string | |
| text: string | |
| alertText: string | |
| infoBg: string | |
| infoBorder: string | |
| inputBg: string | |
| placeholderText: string | |
| } | |
| export interface CedarlingConfigThemeColors { | |
| cardBg: string | |
| navbarBorder: string | |
| text: string | |
| alertText: string | |
| infoBg: string | |
| infoBorder: string | |
| inputBg: string | |
| placeholderText: string | |
| } |
🤖 Prompt for AI Agents
In `@admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.style.ts`
around lines 7 - 16, The CedarlingConfigThemeColors interface is not exported,
so consumers cannot type the themeColors parameter passed to useStyles; export
the interface by adding an export modifier to CedarlingConfigThemeColors (so
callers can import and use that type when constructing themeColors for
useStyles) and ensure any related exports (if you have a default export pattern)
remain consistent with the module's API.
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.style.ts
Outdated
Show resolved
Hide resolved
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.style.ts
Show resolved
Hide resolved
|
Contributor
There was a problem hiding this comment.
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/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx (1)
156-166:⚠️ Potential issue | 🟠 MajorSync runs unconditionally regardless of retrieval point or URL validity.
After saving the configuration,
syncRoleToScopesMappingsMutation.mutateAsync()executes regardless of whethercedarlingPolicyStoreRetrievalPointis'default'or iftrimmedUrlis empty. Based on learnings, the sync operation depends onauiPolicyStoreUrl, so syncing without a valid URL may fail or produce unexpected results.Consider guarding the sync to only run when there's a valid URL to sync from.
🛠️ Proposed guard
payload: requestData, }) + if (trimmedUrl) { await syncRoleToScopesMappingsMutation.mutateAsync() userMessage = t('documentation.cedarlingConfig.auditSyncRoleToScopesMappings') await logAudit({ userinfo: userinfo ?? undefined, action: UPDATE, resource: ADMIN_UI_CEDARLING_CONFIG, message: userMessage, client_id: client_id, payload: requestData, }) + } dispatch(updateToast(true, 'success'))Based on learnings: "the sync role to scopes mappings operation depends on the
auiPolicyStoreUrl. The backend fetches content from this URL and syncs roles based on the response."
🤖 Fix all issues with AI agents
In `@admin-ui/app/routes/Dashboards/DashboardPage.tsx`:
- Around line 96-97: The two separate useAppSelector calls reading
cedarPermissions (cedarInitialized and cedarIsInitializing) should be combined
into a single selector to reduce subscriptions: replace the two calls with one
useAppSelector that returns both fields from state.cedarPermissions (e.g., ({
initialized, isInitializing }) => ({ initialized, isInitializing })) and then
read cedarInitialized and cedarIsInitializing from that single result so
components subscribe once to cedarPermissions.
duttarnab
approved these changes
Feb 5, 2026
moabu
approved these changes
Feb 5, 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 Cedarling Policy, Role & Mapping as per Figma (#2627)
Summary
This PR revamps multiple Admin UI pages to align with the latest Figma designs as part of the ongoing Admin UI redesign initiative.
The updated sections include:
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: #2627
Summary by CodeRabbit
New Features
UI / Style Updates
Internationalization
Developer Experience
Chores