Page MenuHomePhabricator

[landing] introduce differenation section
ClosedPublic

Authored by ginsu on Mon, Jul 1, 7:27 AM.
Tags
None
Referenced Files
F2194927: D12631.diff
Fri, Jul 5, 8:42 AM
Unknown Object (File)
Wed, Jul 3, 11:41 PM
Unknown Object (File)
Tue, Jul 2, 6:02 PM
Unknown Object (File)
Tue, Jul 2, 6:02 PM
Unknown Object (File)
Tue, Jul 2, 6:02 PM
Unknown Object (File)
Tue, Jul 2, 6:02 PM
Unknown Object (File)
Tue, Jul 2, 6:02 PM
Unknown Object (File)
Tue, Jul 2, 6:02 PM
Subscribers

Details

Summary

This diff introduces the new differentiation section on the landing page. Since the copy for this section is not ready atm, I gated this section behind a flag that can easily be enabled/disabled when ready

Depends on D12630

Test Plan

Please see the demo video below

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu requested review of this revision.Mon, Jul 1, 7:44 AM
ginsu edited the test plan for this revision. (Show Details)
ginsu added reviewers: ashoat, inka.
ashoat requested changes to this revision.Mon, Jul 1, 5:50 PM

Wondering, if we wanted to include a graphic how would we do it here? Maybe on the pop-up cards? Is there a way to do that in competitorData currently?

landing/app-landing.react.js
27 ↗(On Diff #41854)

Let's actually do it after the competitor comparison for now

landing/differentiation-info.css
1–44 ↗(On Diff #41854)

It feels like most of this is copy-paste. Can we just use the same CSS file for both components? I'm guessing the CSS is similar enough that it'd be better to share a file

landing/differentiation-info.react.js
56 ↗(On Diff #41854)

Can we say "What makes Comm unique"

This revision now requires changes to proceed.Mon, Jul 1, 5:50 PM

fix feature flag

landing/competitor-comparison.css
62 ↗(On Diff #41866)

The .featureCardsContainer in the DifferentiationInfo does not need a margin-top-48px so moved this style into .competitorsContainer which sits above .featureCardsContainer in CompetitorComparison

https://github.com/CommE2E/comm/blob/master/landing/competitor-comparison.react.js#L88-L89

Wondering, if we wanted to include a graphic how would we do it here?

The best way to go about this would be to add a new general case in competitor-logo.react.js. I initially tried to find a good graphic/icon for this, but wasn't able to find anything really good so I thought it would be better to just not include a graphic.

https://github.com/CommE2E/comm/blob/master/landing/competitor-logo.react.js

Maybe on the pop-up cards?

The CompetitorFeature component is shared between the normal cards and the pop up cards, so if we only want to show a graphic/icon on the pop-up cards we would need to introduce additional logic to make this happen, but it shouldn't be anything too crazy

Let me know if you would like to see me do any of this as a quick follow up

I mean less like an icon, and more like a graphic of the functionality. Eg. for the tree structure piece we could include a screenshot of the navigation side drawer

This revision is now accepted and ready to land.Tue, Jul 2, 11:34 AM

I mean less like an icon, and more like a graphic of the functionality. Eg. for the tree structure piece we could include a screenshot of the navigation side drawer

Oh gotcha. Yea the best way to go about this would be to conditionally render the graphic in competitor-feature.react.js (in between the div with the .headingContainer class name and the CommLogo component).

return (
  <div className={css.container}>
    <div className={css.headingContainer}>
      <p className={headingClassName}>{title}</p>
      {comingSoonBadge}
    </div>
    {/* Introduce conditional graphic here */}
    <CommLogo size={30} />
    {commInfo}
    <hr />
    <CompetitorLogo name={competitorID} size={30} />
    {competitorInfo}
  </div>
);

We should conditionally render this graphic based on some newly introduced props since we probably only want to show the graphic on the larger pop-up cards, and the graphic should use the competitorID prop to determine which graphic should be shown

This revision was landed with ongoing or failed builds.Tue, Jul 2, 1:36 PM
This revision was automatically updated to reflect the committed changes.