Page MenuHomePhabricator

[landing] added learn more about comm link to footer
ClosedPublic

Authored by ginsu on Oct 11 2022, 1:31 PM.
Tags
None
Referenced Files
F3533485: D5348.diff
Wed, Dec 25, 11:36 AM
Unknown Object (File)
Nov 25 2024, 10:34 PM
Unknown Object (File)
Nov 25 2024, 10:34 PM
Unknown Object (File)
Nov 25 2024, 10:34 PM
Unknown Object (File)
Nov 24 2024, 12:38 PM
Unknown Object (File)
Nov 24 2024, 12:37 PM
Unknown Object (File)
Nov 24 2024, 12:37 PM
Unknown Object (File)
Nov 9 2024, 8:10 PM

Details

Summary

Won't land this until the Team Listing part of comm.page has been updated

added learn more about comm link to footer.

Test Plan

Please view the before and after demo to see the changes I made:

Before:

Screen Shot 2022-10-11 at 4.21.37 PM.png (2×3 px, 1 MB)

After:

With margin instead of space:

Screen Shot 2022-10-12 at 11.51.04 AM.png (2×3 px, 1 MB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu edited the test plan for this revision. (Show Details)
ginsu edited the test plan for this revision. (Show Details)
ginsu added reviewers: atul, rohan, abosh.
ginsu requested review of this revision.Oct 11 2022, 1:42 PM
This revision is now accepted and ready to land.Oct 11 2022, 3:09 PM
ginsu added a reviewer: Restricted Owners Package.Oct 11 2022, 7:17 PM
ginsu removed a reviewer: Restricted Owners Package. ginsu added 1 blocking reviewer(s): atul.
This revision now requires review to proceed.Oct 11 2022, 7:18 PM

Code change looks right.

However, the copy ("Learn more about Comm") seems a little too similar to "How Comm works" IMO. Saw that you're holding off on landing this one, so will want to get @ashoat's sign off before that.

landing/footer.react.js
50 ↗(On Diff #17498)

In terms of copy, "Learn more about Comm" seems very close to "How Comm works"... might be confusing what exactly the distinction between the two is. (CC @ashoat on copy)

But the code change looks right.

This revision is now accepted and ready to land.Oct 11 2022, 7:46 PM
landing/footer.react.js
50 ↗(On Diff #17498)

Valid, any suggestion?

landing/footer.react.js
50 ↗(On Diff #17498)

What if we rename "Learn more about Comm" to "About Comm", since it includes details about the company, what the product is, and about the team (and more). Just throwing an idea out there.

landing/footer.react.js
50 ↗(On Diff #17498)

I agree with Rohan; I think we should rename "Learn more about Comm" to "About Comm", and additionally we should rename "How Comm works" to "Learn how Comm works".

landing/footer.react.js
50 ↗(On Diff #17498)

Good suggestions guys, let's do it!

updated text of learn more about comm and how comm works to be more clear

landing/footer.react.js
42 ↗(On Diff #17515)

We should probably use padding or margin instead of a space to separate these, but not a big deal

used margin property instead of space

@atul @ashoat Should we also update the title of the notion pages to reflect the new titles in the footer?