Skip to content

EDM-3449: Display org label in the user dropdown - #667

Merged
celdrake merged 3 commits into
flightctl:mainfrom
celdrake:EDM-3449-display-user-org
Jul 22, 2026
Merged

EDM-3449: Display org label in the user dropdown#667
celdrake merged 3 commits into
flightctl:mainfrom
celdrake:EDM-3449-display-user-org

Conversation

@celdrake

@celdrake celdrake commented Jun 2, 2026

Copy link
Copy Markdown
Collaborator

Always display the user's organization "Display name" (or its ID as a fallback).

When only 1 organization exists, it's displayed as read-only with a popover to be able to see and copy the orgId.
single-org

Update: now using Tooltip and without the "Copy" element:
org-with-tooltip

When multiple organizations exists (with the organization selector)
multiple-orgs

Summary

  • Updated the shared libs/ui-components/ PageNavigation component to always display the user’s organization label in the secondary toolbar/user dropdown.
  • Refactored the organization control into an inline OrganizationDisplay:
    • Single organization: shows a compact, read-only label with a Popover containing a copy-to-clipboard control for the organization ID.
    • Multiple organizations: shows a selector-style dropdown for switching organizations, wired to open the organization selection modal; includes a stable data-testid on the dropdown toggle.
  • Adjusted toolbar alignment in PageNavigation.css to vertically center the organization label consistently (including a new .fctl-subnav_organization rule).

Impact

  • Shared UI component change (libs/ui-components/) affects both:
    • Standalone UI (apps/standalone): PageNavigation rendered from AppLayout.
    • OCP plugin (apps/ocp-plugin): PageNavigation rendered from WithPageLayout (showSettings={false}).
  • No changes indicated to libs/types/, libs/i18n/, libs/cypress/, proxy/, packaging/, or CI/E2E configuration.
  • Cross-cutting behavior change: organization display behavior (single-org popover copy vs multi-org selector/modal) should be validated in both standalone and OCP plugin renders of PageNavigation.

@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

PageNavigation now uses a dedicated organization display for single- and multi-organization states. Single-organization views expose a copyable ID popover, while multi-organization views retain switching behavior. Toolbar alignment is updated for the organization control.

Changes

Organization Display

Layer / File(s) Summary
Organization display behavior
libs/ui-components/src/components/common/PageNavigation.tsx
Adds OrganizationDisplay, showing a single organization’s ID through a Popover and ClipboardCopy, while multi-organization views use the current organization label in the selector toggle.
Toolbar integration and alignment
libs/ui-components/src/components/common/PageNavigation.tsx, libs/ui-components/src/components/common/PageNavigation.css
Renders the organization control from currentOrganization, wires switching to the organization modal, removes derived organization display variables, and vertically centers toolbar content and the organization label.

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

Possibly related PRs

Suggested labels: ui-components, i18n

🚥 Pre-merge checks | ✅ 13 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Unchecked-Errors ⚠️ Warning proxy/ contains unchecked errors: ignored json.Marshal errors and silent swallows of resp.Body.Close()/frontend.WriteControl() with no justification comments. Return, log, or explicitly justify each ignored error; remove silent _ =/unhandled writes in proxy/ and document any cleanup errors that are intentionally safe to ignore.
Generated-Files-Not-Hand-Edited ⚠️ Warning PR directly edited generated file libs/i18n/locales/en/translation.json, adding a new key instead of regenerating it. Regenerate locale assets with npm run i18n and commit the generated output, or move the source change to the correct i18n input file.
✅ Passed checks (13 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
No-Hardcoded-Secrets ✅ Passed No hardcoded secrets found in the touched files; changes only add UI labels, organization ID display, and styling, with no API keys/tokens/passwords or embedded credentials.
No-Weak-Crypto ✅ Passed PR diff only changes UI display/clipboard behavior; no weak crypto, custom crypto, or secret comparisons were added.
No-Injection-Vectors ✅ Passed The PR adds only JSX text/PatternFly rendering; no eval/exec, dangerouslySetInnerHTML, os.system, exec.Command, or yaml.load sinks were introduced.
Container-Privileges ✅ Passed PR only changes UI/i18n files; no container/K8s manifests or privileged settings (privileged, hostPID, hostNetwork, allowPrivilegeEscalation, SYS_ADMIN) are present.
No-Sensitive-Data-In-Logs ✅ Passed No new console/logger calls in touched files; org IDs and login commands are rendered for copy/paste only, not written to logs.
Resource-Leaks ✅ Passed PR diff vs origin/main touches only UI/i18n files; no Go files under proxy/ changed, so the resource-leak check is not applicable.
Ai-Attribution ✅ Passed PR commits include Made-with: Cursor trailers, and no AI-related Co-Authored-By trailer is present.
I18n-Compliance ✅ Passed All user-visible literals in the changed TSX files are wrapped in t(); I found no t() calls with variable keys.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: showing the organization label in the user dropdown.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@celdrake

celdrake commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator Author

This solution only works for the Standalone UI, needs a proper solution for the OCP plugin app.
Update: display the org read-only with a Popover in the secondary toolbar.

@celdrake
celdrake marked this pull request as draft June 16, 2026 12:07
@celdrake
celdrake force-pushed the EDM-3449-display-user-org branch from 9669fa8 to 163e8f4 Compare July 17, 2026 12:41
@celdrake
celdrake marked this pull request as ready for review July 17, 2026 12:42

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@libs/ui-components/src/components/common/PageNavigation.tsx`:
- Line 57: Update the organization display expressions in the
single-organization path and multi-organization toggle, using the organization
ID whenever the optional label is absent. Apply the same fallback pattern in
both locations, including the render around Content and the toggle near the
multi-organization handling, while preserving labels when present.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 07206a19-d961-4a4b-a492-d6ee5de9d5b2

📥 Commits

Reviewing files that changed from the base of the PR and between 9669fa8 and 163e8f4.

⛔ Files ignored due to path filters (1)
  • libs/i18n/locales/en/translation.json is excluded by !libs/i18n/locales/en/translation.json
📒 Files selected for processing (2)
  • libs/ui-components/src/components/common/PageNavigation.css
  • libs/ui-components/src/components/common/PageNavigation.tsx

Comment thread libs/ui-components/src/components/common/PageNavigation.tsx Outdated
@celdrake
celdrake force-pushed the EDM-3449-display-user-org branch from 163e8f4 to f4c9a25 Compare July 17, 2026 12:53
@celdrake
celdrake force-pushed the EDM-3449-display-user-org branch from f4c9a25 to 3690213 Compare July 22, 2026 12:55
@celdrake
celdrake force-pushed the EDM-3449-display-user-org branch from 3690213 to bcecd42 Compare July 22, 2026 13:01

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@libs/ui-components/src/components/common/PageNavigation.tsx`:
- Line 28: Update the imports in PageNavigation.tsx so OrganizationItem is
brought in through a type-only import, while useOrganizationGuardContext remains
a runtime import from OrganizationGuard.
- Around line 48-58: Update the organization Popover trigger in PageNavigation
to use a keyboard-focusable PatternFly Button with a plain or equivalent styling
variant, replacing the non-focusable Content wrapper. Add an accessible name
while preserving the displayed organization.label/default text and existing
popover copy behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 659b3758-21bf-4f1e-8561-a42cde1aed78

📥 Commits

Reviewing files that changed from the base of the PR and between 163e8f4 and 3690213.

⛔ Files ignored due to path filters (1)
  • libs/i18n/locales/en/translation.json is excluded by !libs/i18n/locales/en/translation.json
📒 Files selected for processing (2)
  • libs/ui-components/src/components/common/PageNavigation.css
  • libs/ui-components/src/components/common/PageNavigation.tsx

Comment thread libs/ui-components/src/components/common/PageNavigation.tsx Outdated
Comment thread libs/ui-components/src/components/common/PageNavigation.tsx Outdated
@celdrake

Copy link
Copy Markdown
Collaborator Author

Sorry @jgyselov I had to modify the translations file after rebasing, and there was a new comment about accessibility from Coderabbit. Last commit addresses that: 9f5cb8d

I had to modify from Popover to Tooltip to have the correct "tab key" navigation, and had to drop the "Copy ID" option.
The ID is shown for the purpose of allowing the user to see their ID given that there can be multiple organizations with the same name. So copying the ID is not strictly needing.

@celdrake
celdrake requested a review from jgyselov July 22, 2026 13:28
@celdrake
celdrake merged commit d586f35 into flightctl:main Jul 22, 2026
9 checks passed
@celdrake
celdrake deleted the EDM-3449-display-user-org branch July 22, 2026 13:33
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