feat(@clayui/css): LPD-4556 Convert SVG icons to 16x16 #5810
Conversation
|
For the deprecated icons I would say that we can still keep them and remove them in the future major version, it seems safer, we can now add a warning in the documentation for the deprecated icons. For Lexicon I'm not sure if it needs to update the icon names especially if it's just the file names, I think the designers just use the names in Figma now if the Figma name is different from what exists in Clay I think we can discuss and understand the best option here, mainly because the nomenclature in Clay or Lexicon can be strange so we need to discuss and reach a common sense what we should do about the names you listed. For these icons that don't exist in Lexicon, maybe because they were removed or the name is different? |
The problem is the deprecated icons are 512 x 512. Lexicon will need to create these icons in 16x16.
I checked and they're not in the Figma. One thing I forgot the mention is it's pretty easy to convert 16 x 16 to 512 x 512 from the Figma file. Maybe we don't need to make the 16x16 change. |
Hmm I think we can convert these icons to 16x16 just by changing the size I think there shouldn't be any problems especially if the icons and borders are drawn with just the path instead of the figma resource so decreasing the icon in proportion should work fine.
I don't remember exactly why but why do we want to have 16x16 icons? |
@matuzalemsteles the icons in the Figma Library is 16x16. It's to keep things consistent. The only problem I run into is exporting the whole library. As of now, I need to change the width and height of the icon in the Figma and export them individually. It's not a problem if there's only a few icons we add each time. |
|
@matuzalemsteles I dug a little bit deeper in Portal and it looks like we can interchange 16x16 and 512x512. I'll update this pr and test it. |
|
This is ready for review. I was able to test in most places and it seems to work fine regardless of having |
There was a problem hiding this comment.
@pat270 I only checked a few icons to be precise, there only 52 and it seems that some icons were converted wrong with some errors, there should probably be more like this, I believe that checking the diff in VS Code should be faster than in the browser unfortunately because If there are too many files, the browser starts to slow down and the window crashes...
But basically they seem to be common errors:
- Icon not positioned correctly within the viewbox
- Icon with a larger path than before, for example, big border.
| <svg viewBox="0 0 16 16" xmlns="http://www.w3.org/2000/svg"><path d="m8.792 6.982 1.72-.608L11 7.901l-1.748.593 1.096 1.559-1.229.95-1.126-1.574-1.097 1.573-1.23-.967 1.082-1.542L5 7.9l.473-1.526 1.735.608v-1.98h1.585v1.98Z"/></svg> |
There was a problem hiding this comment.
It seems that the icon is smaller within the 16 16 viewbox when comparing it to the same size.
There was a problem hiding this comment.
@matuzalemsteles this is how it is in the Figma file. We need to ask @drakonux @marcoscv-work.
There was a problem hiding this comment.
Oh okay. I would say that this looks wrong because when the icon size is smaller we will almost not see the icon and this can probably cause visual alignment problems. But let's wait if this is expected.
| <svg viewBox="0 0 16 16" xmlns="http://www.w3.org/2000/svg"><path d="M12.31 2.003H6.231c-.93 0-1.692.771-1.692 1.714V9.76a2.2 2.2 0 0 0-.423-.043c-1.169 0-2.115.959-2.115 2.143 0 1.184.946 2.143 2.115 2.143 1.169 0 2.115-.96 2.115-2.143V6.288h6.077v2.615a2.201 2.201 0 0 0-.423-.043c-1.168 0-2.115.959-2.115 2.143 0 1.184.947 2.142 2.115 2.142 1.17 0 2.116-.958 2.116-2.142V3.717c0-.943-.762-1.714-1.693-1.714Z"/></svg> |
| <svg viewBox="0 0 16 16" xmlns="http://www.w3.org/2000/svg"><path d="m8.672 10.56 4.05-4.11c.793-.844-.285-1.99-1.063-1.165L8 9.008 4.3 5.263c-.721-.768-1.781.415-1.056 1.186l4.06 4.12c.489.532.786.62 1.369-.01Z"/></svg> |
There was a problem hiding this comment.
The border appears larger than it was.
| <svg viewBox="0 0 16 16" xmlns="http://www.w3.org/2000/svg"><path d="m5.44 8.672 4.11 4.05c.844.793 1.99-.285 1.165-1.063L6.992 8l3.745-3.7c.768-.721-.415-1.781-1.186-1.056l-4.12 4.06c-.532.489-.62.786.01 1.369Z"/></svg> |
There was a problem hiding this comment.
The border appears larger than it was.
| <svg viewBox="0 0 16 16" xmlns="http://www.w3.org/2000/svg"><path d="m8.52 12.769 6.247-6.383c.711-.747-.367-1.894-1.084-1.139L8 11.033 2.321 5.251c-.726-.76-1.798.395-1.088 1.14l6.245 6.378c.312.322.754.294 1.041 0Z"/></svg> |
There was a problem hiding this comment.
Icon is not centered in the viewbox.
| <svg viewBox="0 0 16 16" xmlns="http://www.w3.org/2000/svg"><path d="m10.56 7.328-4.11-4.05c-.844-.793-1.99.285-1.165 1.063L9.008 8l-3.745 3.7c-.768.721.415 1.781 1.186 1.056l4.12-4.06c.532-.489.62-.786-.01-1.369Z"/></svg> |
There was a problem hiding this comment.
The border appears larger than it was.
| <svg viewBox="0 0 16 16" xmlns="http://www.w3.org/2000/svg"><path d="m8.672 5.44 4.05 4.11c.793.844-.285 1.99-1.063 1.165L8 6.992l-3.7 3.745c-.721.768-1.781-.415-1.056-1.186l4.06-4.12c.489-.532.786-.62 1.369.01Z"/></svg> |
There was a problem hiding this comment.
The border appears larger than it was.
| <svg viewBox="0 0 16 16" xmlns="http://www.w3.org/2000/svg"><path d="M7.48 3.231 1.235 9.608c-.714.743.358 1.897 1.088 1.142L8 4.967l5.683 5.786c.717.756 1.795-.395 1.084-1.138L8.52 3.23c-.288-.294-.73-.322-1.039 0Z"/></svg> |
There was a problem hiding this comment.
Icon is not centered in the viewbox
| <svg viewBox="0 0 16 16" xmlns="http://www.w3.org/2000/svg"><path d="M15 3.002H1a1 1 0 0 1 0-2h14a1 1 0 0 1 0 2Zm0 6H1a1 1 0 0 1 0-2h14a1 1 0 0 1 0 2Zm-14 6h14a1 1 0 0 0 0-2H1a1 1 0 0 0 0 2Z"/></svg> |
There was a problem hiding this comment.
Icon is not centered in the viewbox
|
Let me review all your comments, @matuzalemsteles. It is a huge task, and without Emi, I'll need to take my time. |
|
@drakonux no problem, don't worry! |
|
@matuzalemsteles @drakonux I reverted the problem icons mentioned above on my local branch. There are no more off center issues. We just need to work on the discrepancies between strokes and icon sizes. |
|
Hey @pat270 and @matuzalemsteles Do we need more info from design to proceed with this PR? |
|
Yes, @matuzalemsteles @pat270, we have completed the task assigned by Emi before their departure. However, I wonder if I've addressed all the issues you mentioned. Could you please review it once more? We can then talk about each issue individually. Disclaimer: Some icons have been modified, which may affect the stroke or spacing. While these changes might appear as bugs, they may not be. |
|
Do you have any updates on this? Is it ready to be merged? Does the design team need to do additional work? cc\ @pat270 |
https://liferay.atlassian.net/browse/LPD-4556
I'm marking this as draft because we have some inconsistencies in naming, missing 16x16 icons, and questions about what to do with deprecated icons. The table below shows the naming conflicts Lexicon has with Clay. Lexicon needs to rename them so it matches Clay due to backward compatibility issues. I'm good with Lexicon prefixing with
icon-but the rest of the name should match.This next table lists icons that are in Clay but doesn't exist in the Lexicon Figma Library. They haven't been deprecated.
Lastly, what should we do about deprecated icons? My gut says it's dangerous to remove them, but I'd like opinions on this.
/cc @matuzalemsteles @ethib137 @marcoscv-work