Page MenuHomePhabricator

[landing] add size prop to logo components
ClosedPublic

Authored by ginsu on May 23 2023, 7:06 AM.
Tags
None
Referenced Files
F2089505: D7945.id27229.diff
Sun, Jun 23, 9:04 PM
Unknown Object (File)
Sat, Jun 22, 2:22 PM
Unknown Object (File)
Wed, Jun 19, 10:55 PM
Unknown Object (File)
Wed, Jun 19, 12:48 PM
Unknown Object (File)
Wed, Jun 19, 8:23 AM
Unknown Object (File)
Wed, Jun 19, 2:22 AM
Unknown Object (File)
Sun, Jun 16, 6:52 PM
Unknown Object (File)
Sat, Jun 15, 5:44 PM
Subscribers

Details

Summary

In the figma designs we have different sizes for the competitor logos. For example in feature cards the logos are 30x30px but in the comparison selector component the logos are 50x50px. To make the size of the logos dynamic I added an optional size prop to each component. If the size field is not passed then we will default the size of the logo to 50x50 (the original size from Figma)

Depends on D7943

Test Plan

Please see the screenshots to see that I was able to change the sizes of the logos

Screenshot 2023-05-23 at 4.16.34 PM.png (2×3 px, 700 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu requested review of this revision.May 23 2023, 7:24 AM

Can we change || to ?? before landing

landing/assets/comm-logo.react.js
9–15 ↗(On Diff #26887)

Because 0 is falsey, if someone passed in size={0}, this would actually display the equivalent of size={50}. Could we instead do size ?? 50?

746a2c.png (602×786 px, 71 KB)

In practice I really doubt it matters since I don't imagine anyone passing in size={0}, but just for correctness I guess.

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

rebase with updated comm logo

This revision was automatically updated to reflect the committed changes.