Skip to content

CONSOLE-5567: Fix PatternFly grid column math in name/value pair editor - #17399

Merged
openshift-merge-bot[bot] merged 2 commits into
openshift:mainfrom
vojtechszocs:CONSOLE-5567-nve-grid-fix
Oct 27, 2026
Merged

openshift-merge-bot[bot] merged 2 commits into
openshift:mainfrom
vojtechszocs:CONSOLE-5567-nve-grid-fix

Conversation

@vojtechszocs

@vojtechszocs vojtechszocs commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to #17398

Analysis / Root cause

PatternFly's Grid always reserves a fixed 12-column track, so GridItem spans in a row must sum to exactly 12 or the unassigned fraction renders as dead space.

From PatternFly design guidelines

The span value of all grid items must equal 12, but this can be achieved by modifying the span of each item to create different combinations of section sizes

The shared name/value pair editor's rows (and their headers) only summed to 11 when editable and 10 when read-only, leaving an unused strip of blank space after the Key/Value (or ConfigMap-Secret/Prefix) fields. This was subtle at narrow modal widths but became clearly visible once the Edit annotations modal was widened (#17398).

Solution description

Column spans are now derived from whether the row's remove-action column is rendered, so every row and its header sum to 12 in both the editable and read-only states.

This editor is shared well beyond the annotations modal - it also backs the container environment variable editor, the storage class parameters form, the PVC creation form and several Knative plugin forms (event source filters, pub/sub filters, the test-function request pane). In all of these, the Key/Value (or equivalent) columns now use the full available width instead of leaving an unexplained empty strip and the "Key"/"Value" (or "ConfigMap/Secret"/"Prefix") column headers now line up correctly above their respective input columns.

This is a PatternFly layout-correctness fix only - no functional or API impact.

Test cases

  • Edit annotations on any resource; confirm no unused blank space after the row inputs and that "Key"/"Value" headers align with their columns.
  • Edit container environment variables on a Deployment/Pod spec; confirm the same alignment, for both plain env vars and "value from ConfigMap/Secret" rows.
  • Create a PVC and a StorageClass with parameters; confirm the key/value rows render correctly.
  • Exercise a read-only rendering of this editor (if applicable in any consumer) to confirm the no-action-column case still sums/aligns correctly.
  • Spot-check a Knative event source or pub/sub filter form that uses this editor.

Summary by CodeRabbit

Summary by CodeRabbit

  • Bug Fixes
    • Improved alignment of name-value pairs, headers, and resource fields in read-only views by giving content more space when action controls are unavailable.
    • In editable views, fields retain a dedicated column for action controls, and add controls remain available. Headers follow the same layout as their corresponding fields.

PatternFly's 12-column Grid requires GridItem spans in a row to sum
to 12, otherwise the unassigned fraction renders as dead space. The
name/value and ConfigMap/Secret pair rows (and their headers) only
summed to 11 when editable and 10 when read-only, leaving unused
space after the Key/Value fields that became more noticeable once
the annotations edit modal was widened. Spans are now computed from
whether the row's remove-action column is rendered, so every row and
its header sum to 12 in both states.
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Oct 19, 2026
@openshift-ci-robot

openshift-ci-robot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

@vojtechszocs: This pull request references CONSOLE-5567 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.1.0" version, but no target version was set.

Details

In response to this:

Follow-up to #17398

Analysis / Root cause

PatternFly's Grid always reserves a fixed 12-column track, so GridItem spans in a row must sum to exactly 12 or the unassigned fraction renders as dead space. The shared name/value pair editor's rows (and their headers) only summed to 11 when editable and 10 when read-only, leaving an unused strip of blank space after the Key/Value (or ConfigMap-Secret/Prefix) fields. This was subtle at narrow modal widths but became clearly visible once the Edit annotations modal was widened (#17398).

Solution description

Column spans are now derived from whether the row's remove-action column is rendered, so every row and its header sum to 12 in both the editable and read-only states.

This editor is shared well beyond the annotations modal - it also backs the container environment variable editor, the storage class parameters form, the PVC creation form and several Knative plugin forms (event source filters, pub/sub filters, the test-function request pane). In all of these, the Key/Value (or equivalent) columns now use the full available width instead of leaving an unexplained empty strip and the "Key"/"Value" (or "ConfigMap/Secret"/"Prefix") column headers now line up correctly above their respective input columns.

This is a PatternFly layout-correctness fix only - no functional or API impact.

Test cases

  • Edit annotations on any resource; confirm no unused blank space after the row inputs and that "Key"/"Value" headers align with their columns.
  • Edit container environment variables on a Deployment/Pod spec; confirm the same alignment, for both plain env vars and "value from ConfigMap/Secret" rows.
  • Create a PVC and a StorageClass with parameters; confirm the key/value rows render correctly.
  • Exercise a read-only rendering of this editor (if applicable in any consumer) to confirm the no-action-column case still sums/aligns correctly.
  • Spot-check a Knative event source or pub/sub filter form that uses this editor.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification

This PR uses the pipeline controller for second-stage tests. Selection and triggering follow the repository configuration.

Use /test ? to list jobs and /pipeline help for commands.

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 71b2b440-70ed-47cd-bf0d-24b66f59c855
📥 Commits

Reviewing files that changed from the base of the PR and between 3befde6 and ae0c9f1.

📒 Files selected for processing: 1
  • frontend/public/components/utils/name-value-editor.tsx
💤 Files with no reviewable changes: 1
  • frontend/public/components/utils/name-value-editor.tsx

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Walkthrough

NameValueEditor and EnvFromEditor adjust value and resource column widths based on read-only state. They omit action columns and controls when actions are unavailable.

Changes

Editor layouts

Layer / File(s) Summary
Conditional value and action columns
frontend/public/components/utils/name-value-editor.tsx
Both editors adjust row and header column spans based on action availability. They conditionally render action columns and add/remove controls.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: logonoff

Merge Risk: ⚪ Minimal · up to ae0c9

This change fixes blank space in the name/value pair editor layout by making column spans total 12. No concrete merge-blocking risk was identified.

Pre-merge checks: Passed 14 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description covers the root cause, solution, affected consumers, and manual test cases. However, it omits required template sections for screenshots or recording, test setup, browser conformance, … Complete every required template section. Add screenshots or a recording for the visual layout change, state the test setup, mark the browsers tested, provide additional information if applicable, identify reviewers and assignees, and repor…
Passed Passed checks: 14 passed
Check name Status Explanation
Title check Passed The title clearly identifies the Jira issue and the primary change: correcting PatternFly grid column spans in the name/value pair editor.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names Passed The pull request changes only frontend/public/components/utils/name-value-editor.tsx. The diff contains no Ginkgo or other test-title constructs such as It(), Describe(), Context(), or When(), and it …
Test Structure And Quality Passed PASS. The pull request changes only frontend/public/components/utils/name-value-editor.tsx. It adds no Ginkgo tests or It blocks, so the listed test-structure requirements do not apply.
Microshift Test Compatibility Passed PASS: The pull request changes only the frontend TypeScript file frontend/public/components/utils/name-value-editor.tsx. It adds no Ginkgo e2e tests or test references to MicroShift-incompatible API…
Single Node Openshift (Sno) Test Compatibility Passed The pull request changes only frontend/public/components/utils/name-value-editor.tsx. The diff adds no Ginkgo e2e tests or other test code, so it introduces no SNO multi-node or HA assumptions.
Topology-Aware Scheduling Compatibility Passed PASS. The pull request changes only frontend/public/components/utils/name-value-editor.tsx. The diff adjusts PatternFly GridItem spans and conditional action controls. It adds or changes no deploy…
Ote Binary Stdout Contract Passed PASS: The pull request changes only frontend/public/components/utils/name-value-editor.tsx, a React/TypeScript layout component. The diff adds grid-span and conditional-rendering logic. It adds no O…
Ipv6 And Disconnected Network Test Compatibility Passed The pull request changes only frontend/public/components/utils/name-value-editor.tsx. The diff contains React/PatternFly layout changes and no new Ginkgo tests or network addresses, URLs, image pull…
No-Weak-Crypto Passed The pull request changes only PatternFly grid spans and action-control rendering in name-value-editor.tsx. The added code contains no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, custom cryptography, o…
Container-Privileges Passed PASS: The pull request changes only frontend/public/components/utils/name-value-editor.tsx, a TypeScript UI component. The diff contains no container or Kubernetes manifest and no privileged, host…
No-Sensitive-Data-In-Logs Passed The pull request changes only PatternFly grid spans and conditional rendering in name-value-editor.tsx. The diff adds no logging calls or log payloads, so it introduces no path that could expose sen…
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.

Full details: Stable And Deterministic Test Names

Explanation

The pull request changes only frontend/public/components/utils/name-value-editor.tsx. The diff contains no Ginkgo or other test-title constructs such as It(), Describe(), Context(), or When(), and it introduces no test names or dynamic title values.

Full details: Microshift Test Compatibility

Explanation

PASS: The pull request changes only the frontend TypeScript file frontend/public/components/utils/name-value-editor.tsx. It adds no Ginkgo e2e tests or test references to MicroShift-incompatible APIs, resources, namespaces, or features.

Full details: Topology-Aware Scheduling Compatibility

Explanation

PASS. The pull request changes only frontend/public/components/utils/name-value-editor.tsx. The diff adjusts PatternFly GridItem spans and conditional action controls. It adds or changes no deployment manifests, operators, controllers, replica settings, affinity, topology spread constraints, node selectors, tolerations, or PDBs. The topology-aware scheduling check is therefore not applicable.

Full details: Ote Binary Stdout Contract

Explanation

PASS: The pull request changes only frontend/public/components/utils/name-value-editor.tsx, a React/TypeScript layout component. The diff adds grid-span and conditional-rendering logic. It adds no OTE binary entry-point code and no stdout writes in process-level code.

Full details: Ipv6 And Disconnected Network Test Compatibility

Explanation

The pull request changes only frontend/public/components/utils/name-value-editor.tsx. The diff contains React/PatternFly layout changes and no new Ginkgo tests or network addresses, URLs, image pulls, or external service access. The IPv6 and disconnected-network test compatibility check is therefore not applicable.

Full details: No-Weak-Crypto

Explanation

The pull request changes only PatternFly grid spans and action-control rendering in name-value-editor.tsx. The added code contains no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, custom cryptography, or secret/token comparisons.

Full details: Container-Privileges

Explanation

PASS: The pull request changes only frontend/public/components/utils/name-value-editor.tsx, a TypeScript UI component. The diff contains no container or Kubernetes manifest and no privileged, host namespace, SYS_ADMIN, allowPrivilegeEscalation, or root-user setting.

Full details: No-Sensitive-Data-In-Logs

Explanation

The pull request changes only PatternFly grid spans and conditional rendering in name-value-editor.tsx. The diff adds no logging calls or log payloads, so it introduces no path that could expose sensitive data in logs.

Full details: Description check

Explanation

The description covers the root cause, solution, affected consumers, and manual test cases. However, it omits required template sections for screenshots or recording, test setup, browser conformance, additional information, and reviewers or assignees. It also reports no test results.

Resolution

Complete every required template section. Add screenshots or a recording for the visual layout change, state the test setup, mark the browsers tested, provide additional information if applicable, identify reviewers and assignees, and report the test results.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@openshift-ci openshift-ci Bot added component/core Related to console core functionality approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 19, 2026
Comment thread frontend/public/components/utils/name-value-editor.tsx Outdated
@logonoff

Copy link
Copy Markdown
Member

/label px-approved
/label docs-approved
/label tide/merge-method-squash

@openshift-ci openshift-ci Bot added px-approved Signifies that Product Support has signed off on this PR docs-approved Signifies that Docs has signed off on this PR tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. labels Oct 19, 2026

@logonoff logonoff left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci

openshift-ci Bot commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: logonoff, vojtechszocs

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Oct 19, 2026
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-gcp-console
/test e2e-gcp-console-techpreview

@vojtechszocs

Copy link
Copy Markdown
Contributor Author

/hold

Will do some more verifications to ensure UX consistency.

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 19, 2026
@vojtechszocs

Copy link
Copy Markdown
Contributor Author

/hold cancel

@openshift-ci openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 21, 2026
@vojtechszocs

Copy link
Copy Markdown
Contributor Author

/test e2e-gcp-console

1 similar comment
@vojtechszocs

Copy link
Copy Markdown
Contributor Author

/test e2e-gcp-console

@logonoff

Copy link
Copy Markdown
Member

/verified by @vojtechszocs

/retest

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Oct 26, 2026
@openshift-ci-robot

Copy link
Copy Markdown
Contributor

@logonoff: This PR has been marked as verified by @vojtechszocs.

Details

In response to this:

/verified by @vojtechszocs

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD f427d32 and 2 for PR HEAD ae0c9f1 in total

@openshift-ci

openshift-ci Bot commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

@vojtechszocs: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot
openshift-merge-bot Bot merged commit b1a26a9 into openshift:main Oct 27, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. component/core Related to console core functionality docs-approved Signifies that Docs has signed off on this PR jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. px-approved Signifies that Product Support has signed off on this PR tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants