feat(OIDC): adjusts for the trust relationships UI - #8313
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan includes up to 8 reviews per rolling hour; 4 remain after this review. 📝 WalkthroughWalkthroughThe trust relationship UI now uses shared provider definitions and reusable provider cards for GitHub and OIDC selection. The modal focuses the first available form field after provider selection. Form controls now include accessible IDs, labels, grouped claim rules, removable role chips, read-only values, and separate loading states. GitHub integration links use Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🔵 Low · up to The PR updates the trust relationships UI and is otherwise mergeable, but one frontend import path still does not follow the repository’s configured alias convention and should be corrected or explicitly accepted by the owner. 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 |
94b0d0b to
daaed40
Compare
740d490 to
a588777
Compare
a588777 to
c80c2b9
Compare
c80c2b9 to
5e82000
Compare
5e82000 to
bd2c46c
Compare
bd2c46c to
55a5746
Compare
Docker builds report
|
❌ private-cloud · depot-ubuntu-latest-arm-16 — run #19517 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Failed testsfirefox › tests/change-request-test.pw.ts › Change Request Tests › Change requests can be created, approved, and published with four-eyes approval @enterprise 🗂️ Previous results✅ private-cloud · depot-ubuntu-latest-16 — run #19520 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19520 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #19520 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-16 — run #19520 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19519 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
✅ private-cloud · depot-ubuntu-latest-16 — run #19519 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
✅ private-cloud · depot-ubuntu-latest-16 — run #19518 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #19519 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-16 — run #19519 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19518 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-16 — run #19518 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
|
Visual Regression15 screenshots compared. See report for details. |
A FieldLabel can only point at a control that has an id, and Switch did not accept one, so its label could not be associated; rc-switch already spreads unknown props onto its <button>, which is labelable. FieldLabel takes an id so an aria-labelledby elsewhere can name a group of controls from it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It defaulted to #000000, so it disappeared on dark surfaces. Defaults to currentColor now, following the text colour in both themes. The one caller that wants white passes it explicitly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7bf484e3-6e2b-4656-a010-9a6072bb0f9e
📒 Files selected for processing (15)
frontend/web/components/Switch.tsxfrontend/web/components/base/forms/FieldLabel.tsxfrontend/web/components/icons/GithubIcon.tsxfrontend/web/components/icons/Icon.tsxfrontend/web/components/pages/organisation-settings/tabs/trust-relationships/GithubTrustRelationshipForm/GithubTrustRelationshipForm.tsxfrontend/web/components/pages/organisation-settings/tabs/trust-relationships/NewTrustRelationshipModal/NewTrustRelationshipModal.scssfrontend/web/components/pages/organisation-settings/tabs/trust-relationships/NewTrustRelationshipModal/NewTrustRelationshipModal.tsxfrontend/web/components/pages/organisation-settings/tabs/trust-relationships/ProviderCard/ProviderCard.scssfrontend/web/components/pages/organisation-settings/tabs/trust-relationships/ProviderCard/ProviderCard.tsxfrontend/web/components/pages/organisation-settings/tabs/trust-relationships/ProviderCard/index.tsfrontend/web/components/pages/organisation-settings/tabs/trust-relationships/TrustRelationshipModal/TrustRelationshipModal.tsxfrontend/web/components/pages/organisation-settings/tabs/trust-relationships/TrustRelationshipPermissionsFields/TrustRelationshipPermissionsFields.tsxfrontend/web/components/pages/organisation-settings/tabs/trust-relationships/TrustRelationships/TrustRelationships.tsxfrontend/web/components/pages/organisation-settings/tabs/trust-relationships/github.tsfrontend/web/components/pages/organisation-settings/tabs/trust-relationships/providers.tsx
Included review availability: Your plan includes up to 8 reviews per rolling hour; 7 remain after this review.
Swaps hand-rolled markup for the components that cover it, and fixes the accessible names those controls were missing: - Role chips become Chip, so removal sits on its own button rather than the whole chip being a delete target, and both ionicons imports go. - Labels become FieldLabel, with htmlFor wiring "Is admin" to its switch. The role chips and claim rules sit in groups named by their label, since a label cannot point at a set of controls. - The read-only InputGroups drop `component`, which bypassed the default Input and the label and error wiring with it. The Select keeps it. - Claim rule rows remove with a trash in the resting-grey square the other removable rows use; close stays for chips and dismissible banners. Each row's inputs and remove button carry its number, rather than three identical placeholders and three buttons reading "Remove rule". - The wildcard hint is referenced with aria-describedby, so it is announced on reaching a values field. - The list's delete button had no accessible name at all; it gains one and drops a data-test nothing referenced. - Both save buttons use isLoading instead of folding the mutation flags into disabled, so they show a spinner. - The integration link becomes Link rather than a Button with an href. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The chooser cards become ProviderCard: leading icon tile, an inline Recommended chip, and a trailing chevron. Its own component rather than an extension of SelectableCard, which stacks its icon above the title and has four consumers, two of which would move. Step two used to give no sign of which provider was picked and no way to change it, since the modal title reads the same on both steps. It now opens with a Provider summary naming the choice, with Change to go back, and focus lands on the first editable field. The provider registry carries the chooser's copy too, so a preset is one entry rather than a label and icon in the registry and the same pair again in the chooser. Its icon became a function of size, since the list badge and the card want different ones, and the GitHub label moved next to GITHUB_ISSUER. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bceb4fa0-c0bd-482a-9de6-d29400a5a163
📒 Files selected for processing (1)
frontend/web/components/pages/organisation-settings/tabs/trust-relationships/providers.tsx
Included review availability: Your plan includes up to 8 reviews per rolling hour; 5 remain after this review.
|
@khvn26 |
docs/if required so people know about the feature.Changes
Follow-up to #8044, against that branch so it lands with the feature.
Chipfor role chips (drops both ionicons imports),FieldLabelfor labels, trash-in-a-square for row removal,Linkfor the integration link,isLoadingon both save buttons.InputGroup'scomponentprop was orphaning labels.ProviderCard, and step two now names the chosen provider with a Change action, since the modal title reads the same on both steps.#000000and vanished in dark mode.How did you test this code?
By hand in both themes, plus keyboard traversal of the claim rule rows. Lint and typecheck clean on the changed files, 449 unit tests pass. No new tests: markup and prop changes only.