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)
Tue, Nov 26, 8:40 PM
Unknown Object (File)
Sat, Nov 23, 11:20 AM
Unknown Object (File)
Sat, Nov 23, 7:06 AM
Unknown Object (File)
Sat, Nov 23, 7:06 AM
Unknown Object (File)
Sat, Nov 23, 7:06 AM
Unknown Object (File)
Oct 28 2024, 10:39 AM
Unknown Object (File)
Oct 19 2024, 9:24 AM
Unknown Object (File)
Oct 19 2024, 9:23 AM
View All 76 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
No Lint Coverage
Unit
No Test Coverage

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

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 28192

View Options

lib/shared/comm-icon-config.json

Loading...
View Options

native/fonts/CommIcons.ttf

Loading...
View Options

web/CommIcon.react.js

Loading...