#3513: Align icons in Blueprints sidebar #4455
Conversation
Codecov Report
@@ Coverage Diff @@
## main #4455 +/- ##
=======================================
Coverage 50.00% 50.00%
=======================================
Files 907 907
Lines 26740 26742 +2
Branches 5440 5440
=======================================
+ Hits 13371 13373 +2
Misses 12442 12442
Partials 927 927
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| @@ -6,4 +6,8 @@ | |||
| font-size: 0.8rem; | |||
| color: #bba8bff5; | |||
| } | |||
|
|
|||
| :global .nav-link { | |||
| padding-inline: 0.7rem; // Reduce horizontal padding | |||
There was a problem hiding this comment.
Reduces the padding inside the selected elements, which at 1rem seems excessive, compared to the available space
| <Nav.Link className="media" {...otherProps}> | ||
| <FontAwesomeIcon className="align-self-center" icon={icon} /> | ||
| <span className="media-body ml-2">{label}</span> | ||
| </Nav.Link> |
There was a problem hiding this comment.
We should use this pattern in more places:
https://getbootstrap.com/docs/4.0/layout/media-object/
Potentially ListItem could be extracted and reused elsewhere.
Note: Nav.Item was unnecessary and was dropped.
There was a problem hiding this comment.
Good point! Never thought to apply this pattern to vertical menus before.
| const ListItem: BsPrefixRefForwardingComponent< | ||
| "a", | ||
| NavLinkProps & { label: string; icon: IconProp } | ||
| > = ({ label, icon, ...otherProps }) => ( |
There was a problem hiding this comment.
I couldn't find a better way to type this.
NavLinkisn't generic and can't be extendedReact.FC<NavLinkProps>for some reason lacks link attributes likehrefReact.FC<NavLink>lackseventKey, which is part of NavLinkPropsReact.FC<NavLink & NavLinkProps>is the same as above
| @@ -157,7 +171,7 @@ const ListFilters: React.FunctionComponent<ListFiltersProps> = ({ | |||
|
|
|||
| return ( | |||
| <Col sm={12} md={3} xl={2} className={styles.root}> | |||
| <Form className="mr-3"> | |||
There was a problem hiding this comment.
This margin was unnecessarily reducing the available horizontal space. Bootstrap’s columns already add a lot of space. Was it added to solve any specific issues?
There was a problem hiding this comment.
Not sure. It was probably me at some point. Could be an artifact of a spacing issue that got resolved. But I don't see any reason to keep it anymore.
What does this PR do?
Before / After
Checklist
ModsPagestory broken(evaluating 'useContext(context).location')#4456