feat: Fix explore dynamic height issues by adding style to ErrorAlertBoundary#32006
Closed
mistercrunch wants to merge 4 commits intomasterfrom
Closed
feat: Fix explore dynamic height issues by adding style to ErrorAlertBoundary#32006mistercrunch wants to merge 4 commits intomasterfrom
mistercrunch wants to merge 4 commits intomasterfrom
Conversation
…Boundary Recently brought in an intricate issue while trying to style the ErrorBoundary. Somehow was triggering a loop of auto-height-expanding. Here my solution is to pass a css prop to the boundary, which carries through to the `Alert` component
mistercrunch
commented
Jan 28, 2025
| <Route path={path} key={path}> | ||
| <Suspense fallback={<Fallback />}> | ||
| <Layout.Content> | ||
| <div style={{ padding: '16px' }}> |
Member
Author
There was a problem hiding this comment.
that was the issue here ^^^
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Fix Detected |
|---|---|---|
 |
Unsafe Theme Property Access ▹ view | ✅ |
Need a new review? Comment
/korbit-reviewon this PR and I'll review your latest changes.Korbit Guide: Usage and Customization
Interacting with Korbit
- You can manually ask Korbit to review your PR using the
/korbit-reviewcommand in a comment at the root of your PR.- You can ask Korbit to generate a new PR description using the
/korbit-generate-pr-descriptioncommand in any comment on your PR.- Too many Korbit comments? I can resolve all my comment threads if you use the
/korbit-resolvecommand in any comment on your PR.- Chat with Korbit on issues we post by tagging @korbit-ai in your reply.
- Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.
Customizing Korbit
- Check out our docs on how you can make Korbit work best for you and your team.
- Customize Korbit for your organization through the Korbit Console.
Feedback and Support
mistercrunch
commented
Jan 28, 2025
| </ErrorBoundary> | ||
| </div> | ||
| </Layout.Content> | ||
| <ErrorBoundary style={{ margin: '16px' }}> |
Member
Author
There was a problem hiding this comment.
wanted to do theme.sizeUnit * 4 but theme isn't in context here so bailed and went for hard-coded
This was referenced Jan 28, 2025
kgabryje
approved these changes
Jan 28, 2025
Member
|
I think we can close this PR now as #32005 is merged |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Recently brought in an intricate issue while trying to style the ErrorBoundary. Somehow was triggering a loop of auto-height-expanding in the Explore app.
Another related issue was around introducing
Layout.Contentin the App.tsx file, was creating issues with SQL Lab rendering the tab-content.Here my solution is to pass a css prop to the boundary, which carries through to the
Alertcomponent. Here styling the boundary object ultimately styles the Alert, while passing the style props through ErrorAlert.Could implement full emotion interface with
classNameandcssbut decided to go minimalist and only pass what we need (style)