Skip to content

ROX-33571: Improve container display components#19511

Draft
dvail wants to merge 1 commit intomasterfrom
dv/ROX-33571-flatten-nested-expandables
Draft

ROX-33571: Improve container display components#19511
dvail wants to merge 1 commit intomasterfrom
dv/ROX-33571-flatten-nested-expandables

Conversation

@dvail
Copy link
Contributor

@dvail dvail commented Mar 19, 2026

Description

Refactors container display components to improve readability and fix a display bug:

  • Removes nested ExpandableSections from Volumes and Secrets components
  • Replaces with flatter Label + DescriptionList layout
  • Removes unnecessary useState toggle state management
  • Fixes readOnlyRootFilesystem not displaying in SecurityContext
  • Improves spacing and visual hierarchy

This prepares the components for consolidation in subsequent PRs.

User-facing documentation

  • CHANGELOG.md is updated OR update is not needed - UI refactor, no user-facing changes
  • documentation PR is created and is linked above OR is not needed - no docs needed

Testing and quality

  • the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag - UI refactor only
  • CI results are inspected

Automated testing

  • added unit tests - existing component tests cover this
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

  • Manually tested in local deployment
  • Verified all container information still displays correctly
  • Confirmed readOnlyRootFilesystem now appears in security context
  • Checked that Labels properly identify volumes and secrets
  • Existing component tests pass

Part of ROX-33571 container display consolidation stack.

@dvail
Copy link
Contributor Author

dvail commented Mar 19, 2026

@openshift-ci
Copy link

openshift-ci bot commented Mar 19, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 2 issues, and left some high level feedback:

  • In ContainerVolumesInfo, String(volume.readOnly) will render "undefined" when readOnly is not set, whereas the previous implementation defaulted to 'false'; consider preserving the explicit fallback or normalizing undefined/null to 'false' to avoid odd UI output.
  • In ContainerArgumentsInfo, you disable react/no-array-index-key and use a composite of arg and index for the key; if args are guaranteed to be unique, using arg alone as the key would avoid the lint override and produce more stable keys.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `ContainerVolumesInfo`, `String(volume.readOnly)` will render `"undefined"` when `readOnly` is not set, whereas the previous implementation defaulted to `'false'`; consider preserving the explicit fallback or normalizing undefined/null to `'false'` to avoid odd UI output.
- In `ContainerArgumentsInfo`, you disable `react/no-array-index-key` and use a composite of `arg` and `index` for the key; if `args` are guaranteed to be unique, using `arg` alone as the key would avoid the lint override and produce more stable keys.

## Individual Comments

### Comment 1
<location path="ui/apps/platform/src/Components/ContainerVolumesInfo.tsx" line_range="54-60" />
<code_context>
-                                    <DescriptionList isCompact className="pf-v6-u-p-md">
-                                        <DescriptionListGroup>
-                                            <DescriptionListTerm>Source</DescriptionListTerm>
-                                            <DescriptionListDescription>
-                                                {secret.path}
-                                            </DescriptionListDescription>
</code_context>
<issue_to_address>
**issue (bug_risk):** Rendering `readOnly` directly as `String(volume.readOnly)` may surface `"undefined"` for missing values.

Previously, falsy values defaulted to `'false'`, so absent `readOnly` didn’t render as `
</issue_to_address>

### Comment 2
<location path="ui/apps/platform/src/Components/ContainerArgumentsInfo.tsx" line_range="18-19" />
<code_context>
         <Card>
             <CardTitle>Arguments</CardTitle>
             {args.length > 0 ? (
-                <CardBody className="pf-v6-u-pt-lg pf-v6-u-mx-lg pf-v6-u-mb-lg">
-                    <List isPlain className="pf-v6-u-max-height" style={styleConstant}>
-                        {args.map((arg) => (
-                            <ListItem>--{arg}</ListItem>
</code_context>
<issue_to_address>
**issue:** Removing the scroll and max-height behavior may let long argument lists grow unbounded.

With the new plain `<CardBody>` and `<List>`, a large number of arguments could make this card (or the whole page) excessively tall. If that’s not intended, it may be worth reintroducing a max-height with overflow or another layout constraint.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines -18 to -19
<CardBody className="pf-v6-u-pt-lg pf-v6-u-mx-lg pf-v6-u-mb-lg">
<List isPlain className="pf-v6-u-max-height" style={styleConstant}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: Removing the scroll and max-height behavior may let long argument lists grow unbounded.

With the new plain <CardBody> and <List>, a large number of arguments could make this card (or the whole page) excessively tall. If that’s not intended, it may be worth reintroducing a max-height with overflow or another layout constraint.

@rhacs-bot
Copy link
Contributor

rhacs-bot commented Mar 19, 2026

Images are ready for the commit at 6592f45.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.11.x-385-ge1376fb365.

@dvail dvail force-pushed the dv/ROX-33571-flatten-nested-expandables branch from b9617b8 to e1376fb Compare March 19, 2026 21:00
@dvail dvail force-pushed the dv/ROX-33571-flatten-nested-expandables branch from e1376fb to 6592f45 Compare March 19, 2026 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants