UPSTREAM: <carry>: fix(frontend): add title tooltip to truncated parameter names in DetailsTable - #404
UPSTREAM: <carry>: fix(frontend): add title tooltip to truncated parameter names in DetailsTable#404jira-autofix[bot] wants to merge 3 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
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:
📝 WalkthroughWalkthrough
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 9 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (9 passed)
Comment |
Commit Checker results: |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/src/components/DetailsTable.tsx (1)
32-34: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGuarantee that long keys can shrink before ellipsis is applied.
This flex item has
flex: 0 0 50%but nominWidth: 0; its automatic minimum may remain content-sized, allowing an unbroken key to widen the row instead of truncating. Verify this in supported browsers and add the explicit minimum-width override if needed.Proposed fix
key: { color: color.strong, flex: '0 0 50%', fontWeight: 'bold', + minWidth: 0, overflow: 'hidden', textOverflow: 'ellipsis', whiteSpace: 'nowrap',🤖 Prompt for 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. In `@frontend/src/components/DetailsTable.tsx` around lines 32 - 34, Add minWidth: 0 to the flex item style in DetailsTable so long, unbroken keys can shrink within their 50% allocation and trigger the existing ellipsis behavior; verify the resulting truncation in supported browsers.frontend/src/components/DetailsTable.test.tsx (1)
22-34: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a regression test for the actual truncation styles.
This mock discards every CSS declaration and returns only plain class names, so the updated snapshots cannot detect removal or misapplication of
overflow,textOverflow, orwhiteSpace. Add a focused test that captures the stylesheet input or runs with real typestyle and verifies the key rule.🤖 Prompt for 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. In `@frontend/src/components/DetailsTable.test.tsx` around lines 22 - 34, The typestyle mock in the DetailsTable tests discards CSS declarations, so snapshots cannot verify truncation styling. Update the mock or add a focused test around the stylesheet mock to capture its input and assert the relevant rule includes overflow, textOverflow, and whiteSpace declarations, or run that test with real typestyle; ensure the regression test detects removal or misapplication of the truncation styles.
🤖 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.
Nitpick comments:
In `@frontend/src/components/DetailsTable.test.tsx`:
- Around line 22-34: The typestyle mock in the DetailsTable tests discards CSS
declarations, so snapshots cannot verify truncation styling. Update the mock or
add a focused test around the stylesheet mock to capture its input and assert
the relevant rule includes overflow, textOverflow, and whiteSpace declarations,
or run that test with real typestyle; ensure the regression test detects removal
or misapplication of the truncation styles.
In `@frontend/src/components/DetailsTable.tsx`:
- Around line 32-34: Add minWidth: 0 to the flex item style in DetailsTable so
long, unbroken keys can shrink within their 50% allocation and trigger the
existing ellipsis behavior; verify the resulting truncation in supported
browsers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 7d16b673-7946-47f9-b802-f805cab73763
📒 Files selected for processing (2)
frontend/src/components/DetailsTable.test.tsxfrontend/src/components/DetailsTable.tsx
|
A set of new images have been built to help with testing out this PR: |
|
An OCP cluster where you are logged in as cluster admin is required. The Data Science Pipelines team recommends testing this using the Data Science Pipelines Operator. Check here for more information on using the DSPO. To use and deploy a DSP stack with these images (assuming the DSPO is deployed), first save the following YAML to a file named apiVersion: datasciencepipelinesapplications.opendatahub.io/v1
kind: DataSciencePipelinesApplication
metadata:
name: pr-404
spec:
dspVersion: v2
apiServer:
image: "quay.io/opendatahub/ds-pipelines-api-server:pr-404"
argoDriverImage: "quay.io/opendatahub/ds-pipelines-driver:pr-404"
argoLauncherImage: "quay.io/opendatahub/ds-pipelines-launcher:pr-404"
persistenceAgent:
image: "quay.io/opendatahub/ds-pipelines-persistenceagent:pr-404"
scheduledWorkflow:
image: "quay.io/opendatahub/ds-pipelines-scheduledworkflow:pr-404"
mlmd:
deploy: true # Optional component
grpc:
image: "quay.io/opendatahub/mlmd-grpc-server:latest"
envoy:
image: "registry.redhat.io/openshift-service-mesh/proxyv2-rhel8:2.3.9-2"
mlpipelineUI:
deploy: true # Optional component
image: "quay.io/opendatahub/ds-pipelines-frontend:pr-404"
objectStorage:
minio:
deploy: true
image: 'quay.io/opendatahub/minio:RELEASE.2019-08-14T20-37-41Z-license-compliance'Then run the following: cd $(mktemp -d)
git clone git@github.com:opendatahub-io/data-science-pipelines.git
cd data-science-pipelines/
git fetch origin pull/404/head
git checkout -b pullrequest b3bb0ff17a73f2fd92d912760a69716727746358
oc apply -f dspa.pr-404.yamlMore instructions here on how to deploy and test a Data Science Pipelines Application. |
Commit Checker results: |
|
Change to PR detected. A new PR build was completed. |
…ng in DetailsTable Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
a937636 to
6d3bb64
Compare
Commit Checker results: |
|
Change to PR detected. A new PR build was completed. |
…meter names in DetailsTable Add title attribute to key spans so users can see the full parameter name on hover when the text is truncated by ellipsis overflow. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Commit Checker results: |
…ier formatting in DetailsTable Fix TypeScript compilation errors (TS2322) at lines 94 and 117 where the title attribute received `string | Element | undefined` but expected `string | undefined`. Extract a `keyTitle` variable that narrows the type via typeof check. Also fix Prettier formatting by breaking the JSX span elements with title attributes into multi-line format matching the project's established pattern. RHOAIENG-56506 Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Commit Checker results: |
|
Change to PR detected. A new PR build was completed. |
Summary
maxWidth: 300hard capmaxWidthwithoverflow: hidden,text-overflow: ellipsis, andwhite-space: nowrapfor proper text truncationtitleattribute to key spans so truncated parameter names show full text on hoverkeyTitlevariable sinceKeyValuekey type includesReact.JSX.ElementTest plan
npm run typecheck)npm run format:check)Fixes: RHOAIENG-56506
Summary by CodeRabbit
New Features
Bug Fixes
Tests