fix(dashboard): embedded dashboard box sizing

[chmelevskij]

SUMMARY

Added correct box-sizing for embedded dashboard elements. This fixes overlaps when embedding.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

After:

TESTING INSTRUCTIONS

Embed one of the dashboards in external application.

ADDITIONAL INFORMATION

• [x] Has associated issue: Fixes #36204, Fixes #35900, Fixes #34830
• [ ] Required feature flags:
• [x] 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

[bito-code-review[bot]]

Code Review Agent Run #899e24

Actionable Suggestions - 0

Review Details

• Files reviewed - 2 · Commit Range: 4851b5e..4851b5e

  • superset-frontend/src/dashboard/containers/DashboardPage.tsx
  • superset-frontend/src/dashboard/styles.ts
• Files skipped - 0
• Tools

  • Eslint (Linter) - ✔︎ Successful
  • 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 Default Agent You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[rusackas]

That's a bit of a CSS shotgun, I'm just not sure if it won't have downstream effects where we were expecting things to be content-box for proper layout.

It might just work, but I wonder if there's not a better way to scope it to just the embedded SDK, or (maybe) be a little more surgical in the approach, adding it to the right components.

If we do go with this approach, we can probably remove a lot of border-box lines of styling throughout the codebase, but again, I'm just not sure where we want content-box

[rusackas]

Just catching up on the thread in https://github.com/apache/superset/issues/36204 - that seems to have a more scoped approach to the styling, if it happens to work. ¯\_(ツ)_/¯

[chmelevskij]

It does feel a bit aggressive, but reasoning behind this:

1. It's quite common snippet in normalize/reset css scripts.
2. Everything inside of ant design Layout already has this applied actually, and in most cases it takes priority over component level css due to specificity of the selectors.

Alternatives would be to put it either in DashboardBuilder styles OR using .dashboard class name in embedded stylings. Open to both if you think that's better option.

[rusackas]

If that's true about <Layout> using border-box in the same way, shouldn't that make things work already?

[rusackas]

I'm just worried about breaking anything that might be using content-box. Times like this, I wish we had screenshot regression testing everywhere.

I think I'd be happy to merge this if the selector could be scoped down to be more specific, if indeed most of our stuff is within <Layout> so we can be a bit more "surgical" (and performant) here.

[chmelevskij]

If that's true about using border-box in the same way, shouldn't that make things work already?

I'm assuming that's a question about Layout. Ant design Layout is only used for the main application, but not for embedded.

I've looked if it's possible to add Layout but it doesn't seem to fit the usecase, or at least would need way much more rethinking to get it work.

I will update it to scope to .dashboar thne.