fix: Big Number side cut fixed #31407
Conversation
fardin-developer
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Updated the PR description to correctly link to and close the issue. |
sadpandajoe
changed the title
geido
added
the
preset:bounty
|
/testenv up |
|
@fardin-developer thanks for the contribution! Could you provide some repro steps? TBH I haven't seen that bug in quite some time |
| color: numberColor, | ||
| marginBottom: '10px', |
There was a problem hiding this comment.
is this additional margin needed here? If so, can we use theme.gridUnit * 2 instead?
There was a problem hiding this comment.
Yes @kgabryje we need margin-bottom here to give some space between header and subheader. You are right we can use theme.gridUnit * 2
I've made the change.
There was a problem hiding this comment.
@kgabryje
Please review my pull request and suggest any additional changes if needed!
pull upgraded codes
|
To repro the same bug:
|
There was a problem hiding this comment.
@fardin-developer Thanks for this. Testing locally works. I have just some question about the prop being removed.
| @@ -120,7 +120,7 @@ class BigNumberVis extends PureComponent<BigNumberVizProps> { | |||
| className="kicker" | |||
| style={{ | |||
| fontSize, | |||
| height: maxHeight, | |||
| height: 'auto', | |||
There was a problem hiding this comment.
Do we know what was the intended use of this maxHeight?
There was a problem hiding this comment.
The maxHeight is used here for two purposes:
- To calculate the fontSize dynamically, ensuring the text fits well.
- To set the height of the div elements, like .header-line and .subheader-line.
Since these div elements only contain text, their height could automatically adjust based on the calculated fontSize. However, maxHeight is still important because it helps determine what the fontSize should be.
There was a problem hiding this comment.
https://painted-tune-041.notion.site/Results-1664127ab6d4806dbab9f3316356e257
Here I have uploaded all the changes and results related to updating height: maxHeight to height: auto.
Let me know if you’d like more details or have any suggestions.
superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx
Outdated
Show resolved
Hide resolved
SUMMARY
Fixes #31010
BEFORE/AFTER SCREENSHOTS
Before
After
Solution