mikebridge commented on code in PR #45476:
URL: https://github.com/apache/superset/pull/45476#discussion_r4331545577
##########
superset-frontend/src/utils/softDeleteCopy.ts:
##########
@@ -164,3 +166,83 @@ export function recoveredToast(
options: { allowHtml: true },
};
}
+
+type ArchivedObjectType = 'chart' | 'dashboard' | 'dataset';
+
+/** Build a collection URL beneath the configured application root. */
+function archiveHref(type: ArchivedObjectType): string {
+ return ensureAppRoot(`/archived/?type=${type}`);
+}
+
+/** Confirmation text with a direct route to the permitted archive collection.
*/
+export function archiveConfirmContent(
+ type: ArchivedObjectType,
+ typeLabel: string,
+ plural = false,
+): ReactElement {
+ return createElement(
+ Fragment,
+ null,
+ archiveConfirmDescription(typeLabel, plural),
+ ' ',
+ createElement('a', { href: archiveHref(type) }, t('Recently Archived')),
+ );
+}
+
+/** An escaped archive toast with a collection link; hard-delete copy stays
plain. */
+export function deletedToastContent(
+ name: string,
+ type: ArchivedObjectType,
+): ToastContent {
+ const text = deletedToast(name);
+ return isFeatureEnabled(FeatureFlag.SoftDelete)
+ ? {
+ text: `${escape(text)} <a
href="${escape(archiveHref(type))}">${escape(t('Recently Archived'))}</a>`,
+ options: { allowHtml: true },
+ }
+ : { text };
+}
+
+/** Isolate the asset name so its direction cannot move translated
punctuation. */
+export function archiveTitle(name: string): ReactElement {
+ const marker = '\uFFF0';
+ const [before, after] = t('Archive %(name)s?', { name: marker }).split(
+ marker,
+ );
+ return createElement(
+ Fragment,
+ null,
+ before,
+ createElement('bdi', { dir: 'auto' }, name),
+ after,
+ );
+}
+
+/** Explain dataset dependencies only when some dependent objects exist. */
+export function datasetArchiveImpact(
+ typeLabel: string,
+ chartCount: number,
+ dashboardCount: number,
+ plural = false,
+): string | null {
+ if (chartCount === 0 && dashboardCount === 0) return null;
Review Comment:
Thanks, Matt. Fixed in 45bfafb8a3: datasets with no dashboard dependencies
use charts-only wording, for both single and bulk archive. Added
singular/plural regressions.
##########
superset-frontend/src/utils/softDeleteCopy.ts:
##########
@@ -164,3 +166,83 @@ export function recoveredToast(
options: { allowHtml: true },
};
}
+
+type ArchivedObjectType = 'chart' | 'dashboard' | 'dataset';
+
+/** Build a collection URL beneath the configured application root. */
+function archiveHref(type: ArchivedObjectType): string {
+ return ensureAppRoot(`/archived/?type=${type}`);
+}
+
+/** Confirmation text with a direct route to the permitted archive collection.
*/
+export function archiveConfirmContent(
+ type: ArchivedObjectType,
+ typeLabel: string,
+ plural = false,
+): ReactElement {
+ return createElement(
+ Fragment,
+ null,
+ archiveConfirmDescription(typeLabel, plural),
+ ' ',
+ createElement('a', { href: archiveHref(type) }, t('Recently Archived')),
Review Comment:
Thanks, agreed. 45bfafb8a3 keeps the confirmation as plain text and removes
the repeated label. The link remains in the success toast after archiving. The
helper still accepts the unused collection argument; removing that is separate
cleanup.
##########
superset-frontend/src/features/charts/ChartCard.test.tsx:
##########
@@ -167,7 +176,7 @@ test('with soft delete on, the card delete flow shows the
archive dialog', async
fireEvent.click(await screen.findByText('Archive'));
const dialog = await screen.findByRole('dialog');
- expect(within(dialog).getByText('Archive Sample
Chart?')).toBeInTheDocument();
+ expect(dialog).toHaveAccessibleName('Archive Sample Chart ?');
Review Comment:
Thanks. Updated both assertions in 45bfafb8a3 to check the title text
content without pinning jsdom's extra space. The BDI direction checks remain. A
tolerant accessible-name assertion is still a useful follow-up.
##########
superset-frontend/src/features/charts/ChartCard.tsx:
##########
@@ -213,21 +218,25 @@ export default function ChartCard({
)
}
>
- <button
- type="button"
- css={menuItemButtonCss}
- data-test="chart-list-delete-option"
- className="action-button"
- onClick={allowEdit ? confirmDelete : undefined}
- >
- <Icons.DeleteOutlined
- iconSize="l"
- css={css`
- vertical-align: text-top;
- `}
- />{' '}
- {deleteActionLabel()}
- </button>
+ <span>
+ <MenuItemButton
+ type="button"
+
Review Comment:
Thanks, cleaned up the blank lines in both cards in 45bfafb8a3.
--
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]
