mikebridge commented on code in PR #43838:
URL: https://github.com/apache/superset/pull/43838#discussion_r4007210656
##########
superset-frontend/src/features/versionHistory/RelatedUpdateRow.tsx:
##########
@@ -121,10 +127,12 @@ export default function RelatedUpdateRow({
</IconWrapper>
<Content>
<Tooltip
- title={rollupEntityNames.map((name, index) => (
- // eslint-disable-next-line react/no-array-index-key
- <div key={index}>{name || t('Untitled')}</div>
- ))}
+ title={tooltipNameList(
Review Comment:
Documented in 3feac978cf as a deliberate scope choice: the roll-up tooltip
answers "which entities rolled up", and the per-record affected-chart detail is
surfaced on single-record rows; unioning affected charts across the rolled-up
records is noted in the comment as a possible follow-up rather than an
oversight.
##########
superset-frontend/src/features/versionHistory/RelatedUpdateRow.tsx:
##########
@@ -144,6 +152,13 @@ export default function RelatedUpdateRow({
const headline = relatedHeadline(record);
const entityName = entityDisplayName(record);
+ // The "Dataset used by N charts updated" phrasing summarizes siblings the
+ // row cannot name inline; the impact payload carries them for the hover
+ // detail (sc-119775), mirroring the rolled-up-names tooltip above.
+ const impactCharts = record.impact?.affected_charts ?? [];
Review Comment:
Fixed in 3feac978cf — when `affected_charts` is absent but `charts > 0` (a
backend that predates the field), the tooltip falls back to a count-only line
("4 affected charts"), so the headline never advertises detail the hover cannot
show. Test added for that shape.
##########
superset-frontend/src/features/versionHistory/RelatedUpdateRow.tsx:
##########
@@ -159,20 +174,45 @@ export default function RelatedUpdateRow({
<Icon iconSize="l" />
</IconWrapper>
<Content>
- <Headline>
- {nameIndex >= 0 ? (
- <>
- {headline.slice(0, nameIndex)}
- <NameLink type="button" onClick={() => onOpen?.(record)}>
- {entityName}
- </NameLink>
- {headline.slice(nameIndex + record.entity_name.length)}
- </>
- ) : (
- headline
- )}
- {record.entity_deleted && ` (${t('deleted')})`}
- </Headline>
+ <Tooltip
Review Comment:
Fixed in 3feac978cf — the `Tooltip` is now mounted only when there is a
title (names, overflow, or the count fallback); the bare `Headline` renders
otherwise, so the common no-impact row is as light as before.
--
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]
