Skip to content

Comments

fix(dataset-modal): fix folders tab scrollbar by establishing proper flex chain#38123

Merged
kgabryje merged 1 commit intoapache:masterfrom
kgabryje:fix-dataset-modal-folders-scrollbar
Feb 20, 2026
Merged

fix(dataset-modal): fix folders tab scrollbar by establishing proper flex chain#38123
kgabryje merged 1 commit intoapache:masterfrom
kgabryje:fix-dataset-modal-folders-scrollbar

Conversation

@kgabryje
Copy link
Member

SUMMARY

The DatasourceModal's CSS selectors targeted Bootstrap-era class names (.modal-content, .modal-body, etc.) instead of antd v5's .ant-modal-* classes, so the intended 900px height and flex layout were never applied. This left the modal body without a definite height, forcing FoldersEditor to use a hardcoded 70vh that caused an unnecessary scrollbar.

Fix the selectors and establish a complete flex chain from the modal body through DatasourceContainer and Tabs down to FoldersContainer, which can now use height: 100% instead of the viewport-relative workaround.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

  1. Open dataset modal
  2. Verify that the modal has a fixed height
  3. Verify that there's no unnecessary scrollbar in Folders tab

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

…flex chain

The DatasourceModal's CSS selectors targeted Bootstrap-era class names
(.modal-content, .modal-body, etc.) instead of antd v5's .ant-modal-*
classes, so the intended 900px height and flex layout were never applied.
This left the modal body without a definite height, forcing FoldersEditor
to use a hardcoded 70vh that caused an unnecessary scrollbar.

Fix the selectors and establish a complete flex chain from the modal body
through DatasourceContainer and Tabs down to FoldersContainer, which can
now use height: 100% instead of the viewport-relative workaround.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@bito-code-review
Copy link
Contributor

bito-code-review bot commented Feb 20, 2026

Code Review Agent Run #c42cd0

Actionable Suggestions - 0
Review Details
  • Files reviewed - 3 · Commit Range: e35fa37..e35fa37
    • superset-frontend/src/components/Datasource/DatasourceModal/index.tsx
    • superset-frontend/src/components/Datasource/FoldersEditor/styles.tsx
    • superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@dosubot dosubot bot added the change:frontend Requires changing the frontend label Feb 20, 2026
display: flex;
flex-direction: column;
align-items: stretch;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The modal body is given flex properties, but its parent .ant-modal-content is not a flex container, so the body's flex: 1 1 auto has no effect; this breaks the intended flex chain and can prevent the body (and nested tabs/folders) from correctly filling the 900px height, reintroducing layout/scroll issues. Make .ant-modal-content a flex container with column direction so the body can expand as intended. [logic error]

Severity Level: Major ⚠️
- ❌ Dataset edit modal body does not expand to 900px.
- ⚠️ Folders tab may show unnecessary inner scrollbars.
- ⚠️ Layout inconsistency versus previous flex-based modal behavior.
Suggested change
}
display: flex;
flex-direction: column;
align-items: stretch;
Steps of Reproduction ✅
1. Start the Superset frontend with this PR applied and navigate to any page that opens
the dataset edit modal as described in the PR ("Open dataset modal" – implemented by
`DatasourceModal` in
`superset-frontend/src/components/Datasource/DatasourceModal/index.tsx`).

2. Open the dataset edit modal; in the Elements panel, locate the top-level modal content
element rendered by Ant Design (`.ant-modal-content`) that corresponds to
`StyledDatasourceModal` (defined at lines 47–60 in `index.tsx`).

3. Inspect the computed styles for `.ant-modal-content`: from `StyledDatasourceModal` it
only receives `height: 900px` (line 48) and no `display: flex`, so it remains a non-flex
container; then inspect `.ant-modal-body`, which has `flex: 1 1 auto; display: flex;
flex-direction: column;` from lines 52–55.

4. Because `.ant-modal-content` is not a flex (or grid) container, the `flex: 1 1 auto` on
`.ant-modal-body` has no effect as a flex item; the body's height is determined purely by
its content instead of expanding within the fixed 900px content height, which in practice
causes the inner Datasource editor (including the Folders tab) not to reliably fill the
available vertical space, reintroducing extra inner scrolling/whitespace that this PR is
meant to eliminate.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** superset-frontend/src/components/Datasource/DatasourceModal/index.tsx
**Line:** 50:50
**Comment:**
	*Logic Error: The modal body is given flex properties, but its parent `.ant-modal-content` is not a flex container, so the body's `flex: 1 1 auto` has no effect; this breaks the intended flex chain and can prevent the body (and nested tabs/folders) from correctly filling the 900px height, reintroducing layout/scroll issues. Make `.ant-modal-content` a flex container with column direction so the body can expand as intended.

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.
👍 | 👎

@kgabryje kgabryje merged commit e30a9ca into apache:master Feb 20, 2026
81 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:frontend Requires changing the frontend size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants