codeant-ai-for-open-source[bot] commented on code in PR #41438:
URL: https://github.com/apache/superset/pull/41438#discussion_r3714822483
##########
superset-frontend/playwright/pages/DashboardPage.ts:
##########
@@ -178,4 +255,192 @@ export class DashboardPage {
await menu.selectSubmenuItem('Download', optionText);
return downloadPromise;
}
+
+ /**
+ * Enter dashboard edit mode and wait for the builder side pane to appear.
+ */
+ async enterEditMode(): Promise<void> {
+ const editButton = new Button(
+ this.page,
+ DashboardPage.SELECTORS.EDIT_BUTTON,
+ );
+ await editButton.click();
+ await this.page
+ .locator(DashboardPage.SELECTORS.BUILDER_PANE)
+ .waitFor({ state: 'visible' });
+ }
+
+ /**
+ * The builder side pane's tab bar (Charts / Layout elements).
+ */
+
+ /**
+ * Switch the builder side pane to one of its tabs.
+ * @param tab - 'Charts' (existing slices) or 'Layout elements' (new
components)
+ */
+ private async openBuilderTab(tab: BuilderTab): Promise<void> {
+ // Scoped to `.ant-tabs` because that is the root the shared Tabs component
+ // expects.
+ const builderTabs = new Tabs(
+ this.page,
+ this.page
+ .locator(`${DashboardPage.SELECTORS.BUILDER_PANE} .ant-tabs`)
+ .first(),
+ );
+ await builderTabs.clickTab(tab);
+ }
+
+ /**
+ * Locator for chart-holder components currently placed on the grid.
+ * Markdown components are chart holders too โ use
+ * {@link getMarkdownEditors} when the assertion must exclude them.
+ */
+ getChartHolders(): Locator {
+ return this.page.locator(DashboardPage.SELECTORS.CHART_HOLDER);
+ }
+
+ /**
+ * Drag an existing chart from the Charts pane onto the dashboard grid.
+ * Requires edit mode to be active.
+ * @param sliceName - The slice name to search for and drag
+ */
+ async addChartByName(sliceName: string): Promise<void> {
+ await this.openBuilderTab('Charts');
+ const search = new Input(this.page, DashboardPage.SELECTORS.CHARTS_SEARCH);
+ await search.fill(sliceName);
+ const card = this.page
+ .locator(DashboardPage.SELECTORS.CHART_CARD)
+ .filter({ hasText: sliceName })
+ .first();
+ await card.waitFor({ state: 'visible' });
+ await html5DragAndDrop(this.page, card, this.dropTarget());
+ }
+
+ /**
+ * Drag a new Layout element (by its label) onto the dashboard grid.
+ * Requires edit mode to be active.
+ * @param label - The new-component label, e.g. 'Text / Markdown'
+ */
+ async addLayoutElement(label: LayoutElementLabel): Promise<void> {
+ await this.openBuilderTab('Layout elements');
+ const source = this.page
+ .locator(DashboardPage.SELECTORS.NEW_COMPONENT)
+ .filter({ hasText: label })
+ .first();
+ await source.waitFor({ state: 'visible' });
+ await html5DragAndDrop(this.page, source, this.dropTarget());
+ }
+
+ /**
+ * The grid's empty drop target, which the grid renders while in edit mode.
+ *
+ * Only resolves while the grid is still empty. Dropping a second component
+ * needs a target relative to the already-placed one, not this.
+ */
+ private dropTarget(): Locator {
+ return this.page.locator(DashboardPage.SELECTORS.EMPTY_DROPTARGET).first();
+ }
+
+ /**
+ * Hover the first placed chart-holder and click its delete button (edit
mode).
+ */
+ async deleteChartHolder(): Promise<void> {
+ const holder = this.getChartHolders().first();
+ await holder.hover();
+ const deleteButton = new Button(
+ this.page,
+ holder.locator(DashboardPage.SELECTORS.DELETE_COMPONENT),
+ );
+ await deleteButton.click();
+ }
+
+ /**
+ * Locator for markdown editor components on the grid.
+ */
+ getMarkdownEditors(): Locator {
+ return this.page.locator(DashboardPage.SELECTORS.MARKDOWN_EDITOR);
+ }
+
+ /**
+ * The rendered ace document inside a markdown component. Present only once
+ * the component has entered its editing state.
+ *
+ * Exposed as a locator rather than routed through the `AceEditor` component:
+ * that component reads and writes through `ace.edit(...)` in page context,
+ * which both bypasses the real keystroke path under test and gives up
+ * web-first retries on assertions.
+ *
+ * @param markdownEditor - A locator from {@link getMarkdownEditors}
+ */
+ getMarkdownAceContent(markdownEditor: Locator): Locator {
+ return markdownEditor.locator(DashboardPage.SELECTORS.ACE_CONTENT);
+ }
+
+ /**
+ * Ace's hidden textarea inside a markdown component โ the element that
+ * receives keystrokes.
+ *
+ * @param markdownEditor - A locator from {@link getMarkdownEditors}
+ */
+ getMarkdownAceInput(markdownEditor: Locator): Locator {
+ return markdownEditor.locator(DashboardPage.SELECTORS.ACE_TEXT_INPUT);
+ }
+
+ /**
+ * Click the dashboard title, moving focus off whichever grid component holds
+ * it. Committing a markdown edit needs a click on some other element, and
the
+ * title is the one that is always present regardless of what is on the grid.
+ *
+ * In edit mode the click focuses the title's input. That is a state change,
+ * not a no-op โ but it edits nothing on its own, so it leaves the component
+ * under test untouched.
+ */
+ async blurToDashboardTitle(): Promise<void> {
+ await this.page
+ .locator(DashboardPage.SELECTORS.EDITABLE_TITLE)
+ .first()
+ .click();
+ }
+
+ /**
+ * Drag a grid component's bottom resize handle down by `deltaY` pixels.
+ * Requires edit mode. Uses the mouse because the resize handle is driven by
+ * `react-resizable`, which tracks real pointer movement.
+ *
+ * @param component - The grid component to resize
+ * @param deltaY - Pixels to drag downwards (positive grows the component)
+ * @returns The component's height before and after the drag
+ */
+ async resizeComponent(
+ component: Locator,
+ deltaY: number,
+ ): Promise<{ heightBefore: number; heightAfter: number }> {
+ const boxBefore = await component.boundingBox();
+ if (!boxBefore) {
+ throw new Error('Cannot resize a component that is not visible');
+ }
+
+ const handle = component
+ .locator(DashboardPage.SELECTORS.RESIZE_HANDLE_BOTTOM)
+ .last();
+ const handleBox = await handle.boundingBox();
+ if (!handleBox) {
+ throw new Error('Resize handle is not visible');
+ }
+
+ const startX = handleBox.x + handleBox.width / 2;
+ const startY = handleBox.y + handleBox.height / 2;
+ await this.page.mouse.move(startX, startY);
+ await this.page.mouse.down();
+ // Multiple steps so react-resizable sees a drag rather than a teleport.
+ await this.page.mouse.move(startX, startY + deltaY, { steps: 10 });
+ await this.page.mouse.up();
+
+ const boxAfter = await component.boundingBox();
+ if (!boxAfter) {
Review Comment:
**Suggestion:** The method reads `boundingBox()` immediately after
`mouse.up()`, but the resize-stop handler dispatches the layout update through
React state. The DOM can still have the old height when this measurement runs,
causing `heightAfter` to equal `heightBefore` and making the test
intermittently fail even though the resize completed. Wait for the component
geometry to change before taking the final measurement. [race condition]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ Markdown resize regression test can intermittently fail.
- โ ๏ธ Playwright results become sensitive to rendering timing.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2e5e33a5be4140e789c03b08b87ea9e0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=2e5e33a5be4140e789c03b08b87ea9e0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/playwright/pages/DashboardPage.ts
**Line:** 437:440
**Comment:**
*Race Condition: The method reads `boundingBox()` immediately after
`mouse.up()`, but the resize-stop handler dispatches the layout update through
React state. The DOM can still have the old height when this measurement runs,
causing `heightAfter` to equal `heightBefore` and making the test
intermittently fail even though the resize completed. Wait for the component
geometry to change before taking the final measurement.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41438&comment_hash=3d85cb9cd08f1713c41bfa1ea510f8a22d40e441c6f63967cdc0a7d7e9354747&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41438&comment_hash=3d85cb9cd08f1713c41bfa1ea510f8a22d40e441c6f63967cdc0a7d7e9354747&reaction=dislike'>๐</a>
--
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]
