UI: Fix TreeNode alignment when using a different font #22221
Conversation
bdriguesdev
force-pushed
the
fix/tree-node-alignment
branch
from
6ae38ed
to
0cdf5e7
Compare
bdriguesdev
force-pushed
the
fix/tree-node-alignment
branch
from
0cdf5e7
to
645c7cd
Compare
|
@cdedreuille I would like to assign this to you if you don't mind. |
| export const IconsWrapper: FunctionComponent<{ children?: React.ReactNode }> = ({ children }) => { | ||
| return ( | ||
| <Wrapper> | ||
| <InvisibleText> </InvisibleText> |
There was a problem hiding this comment.
Why do you need this InvisibleText? It should automatically center it in the middle without this line, no?
There was a problem hiding this comment.
The reason is that when the text for the TreeNode breaks in multiple lines the icon needs to be centered with the first line of text on the right side not with the whole container, like the first example here:
To achieve this the only way I found is to wrap the Icon within an element that has a height of just one line of text and align the icon vertically inside it, so it will be aligned with the first line of text of the TreeNode. We can't use a fixed value that represents the height of one line of text because each font can have a different height, to deal with it I used this trick of placing an invisible character to set a height for the container of the icon to be exactly the same as one line of a text.
Makes sense?
Also, we could align the icon in the center of the TreeNode not with the first line of text, that will be way easier and we can remove the InvisibleText
Just for visualization
Aligning with the first line of text:
Aligning with the TreeNode:
| }); | ||
|
|
||
| // Make the content have a min-height equal to one line of text | ||
| export const IconsWrapper: FunctionComponent<{ children?: React.ReactNode }> = ({ children }) => { |
There was a problem hiding this comment.
You can use FC instead of FunctionComponent
There was a problem hiding this comment.
Good to know, this is better 😄
I changed it here: f562120
ndelangen
requested review from
ndelangen,
JReinhold and
tmeasday
as code owners
ndelangen
changed the title
|
What's the status on this one @ndelangen? |
|
I haven't worked on it @cdedreuille |
|
The change seems good to me @ndelangen. I'll approve it but perhaps someone else in the team can review it as well. |
|
@cdedreuille does this look good to you: |
|
Yes I checked everything on Chromatic. All good for me @ndelangen |
Closes #18607
What I did
The spacing to align the icons of TreeNode with the first line of text is hard coded, so when we change the theme's font it will not line up correctly with the text.
In order to be able to adjust this to work with every custom font that the user can specify, we did a trick: we added a blank character to an element that receives the icons to set the min-height to exactly one line of text, so the icons are aligned with TreeNode text.
Before:
After:
How to test
You can reproduce in two ways:
Checklist
MIGRATION.MD
Maintainers
make sure to add the
ci:mergedorci:dailyGH label to it.["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]