Add Conditional Rendering For Feature Preview Menu Item.#9767
Merged
pavinduLakshan merged 6 commits intowso2:masterfrom Mar 14, 2026
Merged
Conversation
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:
📝 WalkthroughWalkthroughIntroduces a new usePreviewFeatures hook and refactors header and FeaturePreviewModal to consume it; header now conditionally renders the Feature Preview UI via canUsePreviewFeatures, and the modal delegates feature listing, selection, toggles, redirection, and CDS toggle logic to the hook. Changes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the Feature Preview modal to centralize preview-feature state/handlers into a reusable usePreviewFeatures hook, and updates Header + modal to use the hook for conditional rendering and feature interactions.
Changes:
- Added
usePreviewFeatureshook to encapsulate preview feature list building, scope-based access, selection state, and toggle handlers. - Refactored
FeaturePreviewModalto consumeusePreviewFeaturesand removed duplicated inline logic/types. - Updated
Headerto conditionally show the Feature Preview menu item/modal based on hook-derived availability.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| features/admin.core.v1/hooks/use-preview-features.ts | Introduces the shared hook that builds accessible preview features, manages selection, and implements CDS toggle/update behavior. |
| features/admin.core.v1/components/modals/feature-preview-modal.tsx | Simplifies modal by delegating feature list, selection, and actions to usePreviewFeatures. |
| features/admin.core.v1/components/header.tsx | Uses usePreviewFeatures to decide whether to render Feature Preview entry points in the header. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
![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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@features/admin.core.v1/components/header.tsx`:
- Around line 687-703: The menu item and modal use different visibility checks
causing the modal to be mountable when the menu is hidden; update
usePreviewFeatures to return a single canonical boolean (e.g.,
canUsePreviewFeatures) and replace all disparate checks (hasPreviewFeatures,
cdsFeatureConfig scope checks, and any direct uses of FeatureGateConstants
gating around the MenuItem and modal mount) with that one boolean; ensure
setShowPreviewFeaturesModal and showPreviewFeaturesModal still control
opening/closing but are only reachable when canUsePreviewFeatures is true so
both the <Show> wrapper (or its replacement) around the MenuItem and the modal
render/use the same gate.
In `@features/admin.core.v1/hooks/use-preview-features.ts`:
- Around line 153-158: The enable branch only assigns CDS_CONSOLE_APP when
system_applications is empty, so update the nextApps logic to always ensure
CDS_CONSOLE_APP is present when enable is true: compute currentApps from
cdsConfig, then if enable is true add CDS_CONSOLE_APP to currentApps if it's not
already included (e.g., push or create a new array with the value only when
absent), otherwise (when enable is false) filter out CDS_CONSOLE_APP as before;
adjust the calculation for nextApps (the variable in this diff) to reflect this
presence-check and de-dup behavior.
- Around line 85-90: The hook currently starts fetching CDS config whenever CDS
is enabled, even if the user lacks update scope; change the enable flag passed
to useCDSConfig so fetching is gated by the same access check: pass
(cdsFeatureConfig?.enabled ?? false) && hasCDSScopes to useCDSConfig (keep
calling useCDSConfig unconditionally to obey hooks rules but compute the boolean
so the request only runs when both cdsFeatureConfig is enabled and hasCDSScopes
is true).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 8e8026cb-819f-43ad-a05e-f03e9cc03bb3
📒 Files selected for processing (3)
features/admin.core.v1/components/header.tsxfeatures/admin.core.v1/components/modals/feature-preview-modal.tsxfeatures/admin.core.v1/hooks/use-preview-features.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
features/admin.core.v1/hooks/use-preview-features.ts (2)
160-164:⚠️ Potential issue | 🟠 MajorEnsure
CONSOLEis always present when CDS is enabled.Enable flow currently preserves existing
system_applicationsas-is when non-empty, soCONSOLEmay remain missing after re-enable.💡 Suggested update
const currentApps: string[] = cdsConfig?.system_applications ?? []; const nextApps: string[] = enable - ? currentApps.length === 0 - ? [ CDS_CONSOLE_APP ] - : currentApps + ? currentApps.includes(CDS_CONSOLE_APP) + ? currentApps + : [ ...currentApps, CDS_CONSOLE_APP ] : currentApps.filter((app: string) => app !== CDS_CONSOLE_APP);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@features/admin.core.v1/hooks/use-preview-features.ts` around lines 160 - 164, When enabling CDS, the current logic sets nextApps to currentApps unchanged when non-empty, which can leave CDS_CONSOLE_APP out; update the assignment for nextApps in use-preview-features.ts so that when enable is true you always include CDS_CONSOLE_APP (e.g., merge CDS_CONSOLE_APP into currentApps if missing) — modify the nextApps computation (using nextApps, enable, currentApps, CDS_CONSOLE_APP) to ensure CDS_CONSOLE_APP is present in the returned array for system_applications.
93-97:⚠️ Potential issue | 🟠 MajorGate CDS config fetch by access scope to avoid unnecessary API calls.
The fetch currently runs whenever CDS is enabled in config, even for users who cannot access the feature. Use the same scope gate used for visibility.
💡 Suggested update
- const { + const shouldFetchCDSConfig: boolean = Boolean(cdsFeatureConfig?.enabled) && hasCDSScopes; + + const { data: cdsConfig, mutate: mutateCDSConfig - } = useCDSConfig(cdsFeatureConfig?.enabled ?? false); + } = useCDSConfig(shouldFetchCDSConfig);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@features/admin.core.v1/hooks/use-preview-features.ts` around lines 93 - 97, The CDS config fetch is only gated by cdsFeatureConfig?.enabled; update the call to useCDSConfig to also require the same access-scope/visibility gate used elsewhere so the API call only runs for users who can access the feature—i.e., change useCDSConfig(cdsFeatureConfig?.enabled ?? false) to useCDSConfig((cdsFeatureConfig?.enabled ?? false) && <the visibility/access gate variable or function used for CDS visibility>) so useCDSConfig and mutateCDSConfig only run when both enabled and accessible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@features/admin.core.v1/components/header.tsx`:
- Around line 687-693: The new menu item uses a literal string for translation
and lacks a component id; update the ListItemText to use a namespaced i18n key
(e.g., "admin:featurePreview" or similar) instead of t("Feature Preview") and
add a data-componentid attribute on the MenuItem (or the clickable element) to
follow conventions; locate the MenuItem that calls setShowPreviewFeaturesModal
and uses PreviewFeaturesIcon/ListItemText and replace the literal key with the
namespaced key and add data-componentid="header-feature-preview-menuitem" (or
repo-standard id) so localization and component identification are consistent.
In `@features/admin.core.v1/hooks/use-preview-features.ts`:
- Around line 75-76: The JSDoc `@returns` text is out of sync: it mentions
hasPreviewFeatures but the hook exposes canUsePreviewFeatures; update the
comment in use-preview-features.ts so the `@returns` description lists the actual
API names (including canUsePreviewFeatures, previewFeatures list, accessible
list, and modal state/handlers) instead of hasPreviewFeatures to accurately
reflect the hook's exported values.
---
Duplicate comments:
In `@features/admin.core.v1/hooks/use-preview-features.ts`:
- Around line 160-164: When enabling CDS, the current logic sets nextApps to
currentApps unchanged when non-empty, which can leave CDS_CONSOLE_APP out;
update the assignment for nextApps in use-preview-features.ts so that when
enable is true you always include CDS_CONSOLE_APP (e.g., merge CDS_CONSOLE_APP
into currentApps if missing) — modify the nextApps computation (using nextApps,
enable, currentApps, CDS_CONSOLE_APP) to ensure CDS_CONSOLE_APP is present in
the returned array for system_applications.
- Around line 93-97: The CDS config fetch is only gated by
cdsFeatureConfig?.enabled; update the call to useCDSConfig to also require the
same access-scope/visibility gate used elsewhere so the API call only runs for
users who can access the feature—i.e., change
useCDSConfig(cdsFeatureConfig?.enabled ?? false) to
useCDSConfig((cdsFeatureConfig?.enabled ?? false) && <the visibility/access gate
variable or function used for CDS visibility>) so useCDSConfig and
mutateCDSConfig only run when both enabled and accessible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: b92d62e1-9bd7-4646-a2dd-1035bc36ac9a
📒 Files selected for processing (2)
features/admin.core.v1/components/header.tsxfeatures/admin.core.v1/hooks/use-preview-features.ts
ImalshaD
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9767 +/- ##
=======================================
Coverage 56.01% 56.01%
=======================================
Files 42 42
Lines 1023 1023
Branches 247 254 +7
=======================================
Hits 573 573
Misses 416 416
Partials 34 34
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
features/admin.core.v1/components/modals/feature-preview-modal.tsx (1)
67-69: MemoizehandleClosewithuseCallback.
handleCloseis recreated on every render; memoizing it aligns with the callback stability rule.💡 Proposed change
-import React, { ChangeEvent, FunctionComponent, ReactElement } from "react"; +import React, { ChangeEvent, FunctionComponent, ReactElement, useCallback } from "react"; - const handleClose: () => void = (): void => { - onClose(); - }; + const handleClose: () => void = useCallback( + (): void => { + onClose(); + }, + [ onClose ] + );As per coding guidelines, "Stabilize callbacks with useCallback and expensive computations with useMemo."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@features/admin.core.v1/components/modals/feature-preview-modal.tsx` around lines 67 - 69, The handleClose function in feature-preview-modal.tsx is recreated each render; wrap it with React.useCallback to stabilize the callback reference (e.g., const handleClose = useCallback(() => onClose(), [onClose])) so it doesn't change unnecessarily; update imports if needed and ensure you reference the existing handleClose and onClose symbols when applying the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@features/admin.core.v1/components/modals/feature-preview-modal.tsx`:
- Around line 59-65: The destructuring of usePreviewFeatures() lacks explicit
types; import UsePreviewFeaturesReturnInterface from the hooks module and
annotate the destructured binding accordingly so the const is typed as
UsePreviewFeaturesReturnInterface (e.g., const { accessibleFeatures,
handlePageRedirection, handleToggleChange, selected, setSelectedFeatureIndex }:
UsePreviewFeaturesReturnInterface = usePreviewFeatures()), ensuring you update
imports to include UsePreviewFeaturesReturnInterface and keep the existing
variable names (accessibleFeatures, handlePageRedirection, handleToggleChange,
selected, setSelectedFeatureIndex).
---
Nitpick comments:
In `@features/admin.core.v1/components/modals/feature-preview-modal.tsx`:
- Around line 67-69: The handleClose function in feature-preview-modal.tsx is
recreated each render; wrap it with React.useCallback to stabilize the callback
reference (e.g., const handleClose = useCallback(() => onClose(), [onClose])) so
it doesn't change unnecessarily; update imports if needed and ensure you
reference the existing handleClose and onClose symbols when applying the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 89ea234a-f1f2-4e92-9caa-353dc5d62a43
📒 Files selected for processing (1)
features/admin.core.v1/components/modals/feature-preview-modal.tsx
pavinduLakshan
approved these changes
Mar 14, 2026
This was referenced Mar 14, 2026
2 tasks
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
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.
This pull request refactors the feature preview modal logic by extracting it into a new reusable hook,
usePreviewFeatures, and updates theHeaderand modal components to use this hook. The changes improve code modularity, readability, and maintainability by centralizing preview feature logic and reducing duplication.Feature Preview Modal Refactoring:
usePreviewFeatures, which encapsulates all logic related to preview features, including accessible features, toggle handlers, and modal state. This replaces the previous inline logic in the modal component.FeaturePreviewModalto use the new hook, removing redundant code and type definitions, and delegating all feature preview logic tousePreviewFeatures. [1] [2]Header Component Updates:
usePreviewFeaturesin theHeadercomponent to determine if preview features are available, and conditionally render the Feature Preview menu item and modal based on this state. [1] [2] [3] [4] [5]Type and Interface Consolidation:
PreviewFeaturesListInterfacedefinition from the modal component to the new hook, reducing duplication and improving type organization. [1] [2]