EnxDev commented on PR #37299:
URL: https://github.com/apache/superset/pull/37299#issuecomment-4851521887
## EnxDev's Review Agent โ apache/superset#37299 ยท HEAD ab55b5a
**comment** โ fix is reasonable, but the description doesn't match the diff,
one open design thread is unresolved, and truncated names lose their tooltip on
2 of 3 lists.
Classified as a UI fix. No correctness/security/authz concerns. Notes below.
### ๐ก Should-fix
- **PR description is stale.** It claims the fix is `scrollbar-gutter:
stable` + `tableLayout="fixed"`, but the diff contains neither โ the actual
mechanism is fixed pixel `width` hints via `size` (`xxxl` = 360 on the Name
column). Since Superset squash-merges the PR body into the commit, this would
document a fix that wasn't made. Update the summary to describe the width-based
approach.
- **`ChartList/index.tsx:371` and `DatasetList/index.tsx:718`** โ With
`TableCollection` on `tableLayout="auto"`
(`packages/.../TableCollection/index.tsx:312`) and `ellipsis: true`, pinning
Name to `width: 360` makes long names truncate at ~360px instead of expanding.
`DashboardList` recovers via `title={dashboardTitle}` on the `Link`, but the
Chart (`<Link>`) and Dataset (`GenericLink`/`Link`) name links have no
`title`/tooltip, so a truncated name is now unreadable with no hover fallback.
Add a `title`/tooltip on those two links.
- **Open reviewer threads not addressed:** msyavuz flagged the favourites
column looking off โ that's the `size: 'xs'โ'sm'` widening of the `FaveStar`
column (`ChartList:355`, `DashboardList:355`); the PR still carries
`requires:design-review` with no designer sign-off. alexandrusoare asked
whether the fixed widths should extend to the Users/Roles/Groups CRUD views.
Both are unanswered.
### ๐ต Nits
- **`TableCollection/utils.tsx:39,50`** โ `xxxl` diverges from AntD's size
scale, which stops at `xxl`; the rest of `COLUMN_SIZE_MAP` mirrors AntD. This
is my earlier thread โ still worth aligning the naming (or documenting why it
extends beyond `xxl`).
Note: `tableLayout` stays `"auto"`, so the new `width`s are floor hints
rather than hard caps. Pinning only the Name column stabilizes the reported
shift, but the unsized columns can still recompute per page โ verify the shift
is fully gone across pagination on the screenshots' viewport before merge.
<!-- enxdev-review-agent:ab55b5a -->
_Reviewed by EnxDev's Review Agent โ @EnxDev ยท HEAD ab55b5a._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
