Page MenuHomePhabricator
Differential D8308

[lib/native] add user-filled to comm icons pack
ClosedPublic
Actions

Authored by ginsu on Jun 23 2023, 11:04 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Jul 15, 3:03 PM
Unknown Object (File)
Fri, Jul 12, 5:03 PM
Unknown Object (File)
Thu, Jul 4, 3:10 PM
Unknown Object (File)
Wed, Jul 3, 2:09 AM
Unknown Object (File)
Tue, Jul 2, 10:11 PM
Unknown Object (File)
Tue, Jul 2, 10:11 PM
Unknown Object (File)
Tue, Jul 2, 10:11 PM
Unknown Object (File)
Tue, Jul 2, 7:36 AM
View All 51 Files
Subscribers
ashoat
• ted
tomek

Details

Reviewers
atul
rohan
Commits
rCOMM6119ef78e56f: [lib/native] add user-filled to comm icons pack
Summary

This diff adds the user-filled icon to the comm icons pack. These changes were copy and pasted from files given to me by @ted

I also added the icons to our dropbox

Test Plan

Please see the screenshot below:

Screenshot 2023-06-23 at 11.02.50 AM.png (136×196 px, 8 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu created this revision.Jun 23 2023, 11:04 AM
Herald added subscribers: tomek, ashoat. · View Herald TranscriptJun 23 2023, 11:04 AM
ginsu edited the summary of this revision. (Show Details)Jun 23 2023, 11:05 AM
ginsu edited the test plan for this revision. (Show Details)
ginsu added reviewers: atul, rohan.
ginsu attached a referenced file: F604099: Screenshot 2023-06-23 at 11.02.50 AM.png. (Show Details)
ginsu published this revision for review.Jun 23 2023, 11:08 AM
Harbormaster completed remote builds in B20473: Diff 28055.Jun 23 2023, 11:36 AM
rohan accepted this revision.Jun 23 2023, 12:06 PM
rohan added 1 blocking reviewer(s): atul.

Thanks! Icon looks good, I just don't know how icon file changes work so adding @atul as blocking

atul accepted this revision.Jun 26 2023, 7:22 AM

Was this based off of comm-icon-config.json from the repo?

It looks like there are changes unrelated to the new icon being added.

that said, accepting to unblock

lib/shared/comm-icon-config.json
409–425 ↗(On Diff #28055)

It looks like there are unrelated changes?

This revision is now accepted and ready to land.Jun 26 2023, 7:22 AM
ginsu added a comment.Jun 26 2023, 10:46 AM

Was this based off of comm-icon-config.json from the repo?

The comm-icon-config.json was copied and pasted from the selection.json in this folder from dropbox

cc @ted

atul requested changes to this revision.Jun 26 2023, 1:08 PM

The comm-icon-config.json was copied and pasted from the selection.json in this folder from dropbox

Shouldn't it be based on what's in the repo?

This revision now requires changes to proceed.Jun 26 2023, 1:08 PM
ginsu updated this revision to Diff 28190.Jun 27 2023, 12:38 PM
ginsu edited the summary of this revision. (Show Details)

address comments

atul accepted this revision.Jun 27 2023, 12:42 PM

Sweet! Could you check web as well before landing?

This revision is now accepted and ready to land.Jun 27 2023, 12:42 PM
Harbormaster completed remote builds in B20556: Diff 28190.Jun 27 2023, 12:55 PM
ginsu updated this revision to Diff 28192.Jun 27 2023, 2:03 PM

add icons CommIcons type on web

ginsu added inline comments.Jun 27 2023, 2:05 PM
web/CommIcon.react.js
20–27 ↗(On Diff #28192)

Forgot to add these icon names to this type when I previously added the new icons, double checked each of these and made sure they look as expected

This revision was landed with ongoing or failed builds.Jun 27 2023, 2:07 PM
Closed by commit rCOMM6119ef78e56f: [lib/native] add user-filled to comm icons pack (authored by ginsu). · Explain Why
This revision was automatically updated to reflect the committed changes.
ginsu added a commit: rCOMM6119ef78e56f: [lib/native] add user-filled to comm icons pack.
Harbormaster completed remote builds in B20558: Diff 28192.Jun 27 2023, 2:23 PM
rohan mentioned this in D8467: [lib/native/web] add user edit icon to comm icon pack.Jul 10 2023, 1:41 PM

Revision Contents
Changeset List

PathSize
lib/
shared/
142 lines
native/
fonts/
web/
7 lines

Diff 28193

View Options

lib/shared/comm-icon-config.json

Loading...
View Options

native/fonts/CommIcons.ttf

Loading...
View Options

web/CommIcon.react.js

Loading...