Page MenuHomePhabricator

[lib/native] add more icons to comm icon pack
ClosedPublic

Authored by ginsu on May 6 2023, 2:59 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 24, 12:39 PM
Unknown Object (File)
Sat, Nov 23, 6:51 PM
Unknown Object (File)
Sat, Nov 23, 6:51 PM
Unknown Object (File)
Sat, Nov 23, 6:50 PM
Unknown Object (File)
Mon, Nov 11, 1:28 AM
Unknown Object (File)
Mon, Nov 11, 1:27 AM
Unknown Object (File)
Tue, Nov 5, 2:11 AM
Unknown Object (File)
Oct 23 2024, 4:24 PM

Details

Summary

This diff adds the following new icons to the CommIcon pack: edit-filled, link_plus-outline, unpin-outline and pin-outline. These changes were copy and pasted from files given to me by Ted.

I also added the icons to our dropbox

Test Plan

Tested every icon to make sure there were no regressions, and here are the screenshots for the new icons

edit-filled

Screenshot 2023-05-06 at 11.53.43 PM.png (116×162 px, 8 KB)

link_plus-outline

Screenshot 2023-05-06 at 11.53.16 PM.png (162×200 px, 15 KB)

unpin-outline

Screenshot 2023-05-06 at 11.53.01 PM.png (174×212 px, 14 KB)

pin-outline

Screenshot 2023-05-06 at 11.52.46 PM.png (138×202 px, 11 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu requested review of this revision.May 6 2023, 3:17 PM

@atul should review this. I'm not familiar with comm-icon-config.json and its format

Does the sizing look right? Can't really tell from the images from the Test Plan, but as long as things looked right during your testing.

This revision is now accepted and ready to land.May 8 2023, 9:15 AM

It'd also be good to test on native to make sure the generated TTF glyphs look as expected.

ginsu planned changes to this revision.May 8 2023, 9:39 AM

Does the sizing look right? Can't really tell from the images from the Test Plan, but as long as things looked right during your testing.

I actually just double checked using the inspector tool, and noticed that these new icons are actually a tad bit smaller, will have ted fix this and send me some new icons

It'd also be good to test on native to make sure the generated TTF glyphs look as expected.

The screenshots in the test plan are all from native

ginsu requested review of this revision.May 8 2023, 11:57 AM

https://linear.app/comm/issue/ENG-3855/make-commicons-all-uniform-at-16x16px

Discussed this with @ashoat and Ted, but we are going to land this for now to unblock @rohan and the task above is a follow up task to make all our comm icons uniform at 16x16px

This revision is now accepted and ready to land.May 8 2023, 11:57 AM
ginsu edited the test plan for this revision. (Show Details)

Realized that I misnamed the edit-filled icon in the description/test plan but @kuba you should be good now!