8000 [BUGFIX] Remove some SX props leaking by Gladorme · Pull Request #2408 · perses/perses · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[BUGFIX] Remove some SX props leaking #2408

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Nov 8, 2024
Merged

[BUGFIX] Remove some SX props leaking #2408

merged 1 commit into from
Nov 8, 2024

Conversation

Gladorme
Copy link
Member
@Gladorme Gladorme commented Nov 8, 2024 •
edited
Loading

Description

Thanks to @andreasgerstmayr comment on one of his PR, I discovered dynamic values in SX is bad: https://mui.com/system/getting-started/the-sx-prop/#dynamic-values and should move to inline style.

It's a first pass on height / width props, but there are probably still some left. The biggest one is still on the EChart component but look like it's used as a trick to re-render the component 🤔

Checklist

  • Pull request has a descriptive title and context useful to a reviewer.
  • Pull request title follows the [<catalog_entry>] <commit message> naming convention using one of the
    following catalog_entry values: FEATURE, ENHANCEMENT, BUGFIX, BREAKINGCHANGE, DOC,IGNORE.
  • All commits have DCO signoffs.

UI Changes

  • Changes that impact the UI include screenshots and/or screencasts of the relevant changes.
  • Code follows the UI guidelines.
  • Visual tests are stable and unlikely to be flaky.
    See Storybook
    and e2e docs for more details. Common issues
    include:
    • Is the data inconsistent? You need to mock API requests.
    • Does the time change? You need to use consistent time values or mock time utilities.
    • Does it have loading states? You need to wait for loading to complete.

@Gladorme
[BUGFIX] Remove some SX props leaking
598ebcd 
Signed-off-by: Guillaume LADORME <Gladorme@users.noreply.github.com>
@Gladorme Gladorme force-pushed the gladorme/fix-leaking-sx branch from d320321 to 598ebcd Compare November 8, 2024 13:26
@Gladorme Gladorme marked this pull request as ready for review November 8, 2024 13:36
AntoineThebaud
AntoineThebaud approved these changes Nov 8, 2024
View reviewed changes
Copy link
Contributor
@AntoineThebaud AntoineThebaud left a comment

Choose a reason for hiding this comment

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

🙏

@Gladorme Gladorme added this pull request to the merge queue Nov 8, 2024
Merged via the queue into main with commit 0dcb0a4 Nov 8, 2024
18 checks passed
@Gladorme Gladorme deleted the gladorme/fix-leaking-sx branch November 8, 2024 14:53
@Gladorme Gladorme mentioned this pull request Nov 28, 2024
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AntoineThebaud AntoineThebaud AntoineThebaud approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Gladorme @AntoineThebaud
0