refactor(collapsible-panel): update icon sizes#1504
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates icon sizes in collapsible-panel and side-panel for consistency, changing them to 24px. The changes involve updating CSS classes from icon to icon-lg in the relevant HTML templates and making a minor layout adjustment. A security audit identified a Medium severity Cross-Site Scripting (XSS) vulnerability related to the si-icon component's use of a dangerous innerHTML binding, and the reviewed files pass potentially untrusted data to it. Although this PR did not introduce the underlying flaw, its modifications to code using this component increase the importance of addressing this security concern.
|
|
|
Documentation. Coverage Reports: |
|
@spike-rabbit @dauriamarco im not sure we should put this in 24px...i guess that you are doing it because the vertical nav has 24px...but, in the case of the side panel it looks odd. Im not sure, @hbxes WDYT? |
There was a problem hiding this comment.
@panch1739 @hbxes @spike-rabbit continuing the conversation here: I don’t necessarily prefer larger icons in general, but when navbar-vertical and side-panel are shown in the same layout (see screenshots), the size difference is noticeable. Using different icon sizes makes spacing and alignment feel slightly inconsistent.
To me, it feels more coherent if both components use the same sizing and spacing, either small in both cases or large in both, rather than mixing the two. But the final call is yours 👌
With the same size:
With different sizes:
Yea, it looks really odd, when the side panel is present, maybe we have to reconsider the navbar and use there also 20px, to keep it consistant |
dauriamarco
added
the
ux
3 participants
Updates the icon size of collapsible-panel to 24px.