Paths

Table of Contentst

[landing] clean up competitor comparison UI
ClosedPublic
Actions

Authored by ginsu on Jul 11 2023, 1:25 PM.

Details

Reviewers

atul
kamil

Commits

rCOMM790ce888a015: [landing] clean up competitor comparison UI

Summary

As I was inputting copy for one of the competitors I came across some modals/elements that were not super clean visaully. Spoke with @ted and he gave me some tips on how to clean things up.

We should make the padding for the modal container 32px all around
The descriptions in the competitor feature comparison modal should not have a min height
The dividers in the competitor feature cards that have are a tad off

Linear task: https://linear.app/comm/issue/ENG-4346/remove-min-height-for-competitor-feature-comparison-modal

Depends on D8176

Test Plan

Please see the screenshots below to see that each point in the summary was addressed:

We should make the padding for the modal container 32px all around

Before:

Screenshot 2023-07-11 at 4.33.20 PM.png (2×3 px, 922 KB)

After:

Screenshot 2023-07-11 at 4.33.34 PM.png (2×3 px, 921 KB)

The descriptions in the competitor feature comparison modal should not have a min height

Before:

Screenshot 2023-07-07 at 4.34.32 PM.png (2×3 px, 916 KB)

After:

Screenshot 2023-07-11 at 4.34.53 PM.png (2×3 px, 860 KB)

The dividers in the competitor feature cards that have are a tad off

Before:

Screenshot 2023-07-11 at 4.13.03 PM.png (2×3 px, 758 KB)

After:

Screenshot 2023-07-11 at 4.35.30 PM.png (2×3 px, 846 KB)

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ginsu created this revision.Jul 11 2023, 1:25 PM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptJul 11 2023, 1:25 PM

ginsu edited the summary of this revision. (Show Details)Jul 11 2023, 1:37 PM

ginsu edited the test plan for this revision. (Show Details)

ginsu added reviewers: atul, kamil.

ginsu added a parent revision: D8176: [landing] fix size prop for icons in mobile nav.

ginsu attached a referenced file: F631916: Screenshot 2023-07-11 at 4.35.30 PM.png. (Show Details)

ginsu attached a referenced file: F631912: Screenshot 2023-07-11 at 4.13.03 PM.png. (Show Details)

ginsu attached a referenced file: F631903: Screenshot 2023-07-11 at 4.34.53 PM.png. (Show Details)

ginsu attached a referenced file: F631902: Screenshot 2023-07-07 at 4.34.32 PM.png. (Show Details)

ginsu attached a referenced file: F631901: Screenshot 2023-07-11 at 4.33.34 PM.png. (Show Details)

ginsu attached a referenced file: F631900: Screenshot 2023-07-11 at 4.33.20 PM.png. (Show Details)

ginsu added inline comments.Jul 11 2023, 1:41 PM

landing/competitor-feature.css
14 ↗	(On Diff #28601)	We use 34px since that is the height of the coming soon badge. Originally the height of the badge was slightly bigger than the height of the text so by setting the container to be the height of the badge everything is now aligned.

Harbormaster completed remote builds in B20879: Diff 28601.Jul 11 2023, 1:43 PM

ginsu requested review of this revision.Jul 11 2023, 1:43 PM

ginsu mentioned this in D8480: [landing] fix body scrolling when modal is active.Jul 11 2023, 3:52 PM

ginsu added a child revision: D8480: [landing] fix body scrolling when modal is active.Jul 11 2023, 3:55 PM

kamil added inline comments.Jul 13 2023, 3:54 AM

landing/competitor-feature-card.css
20 ↗	(On Diff #28601)	I am not sure if I follow why you moved this here and still passed this to the same exact component, where it was previously defined directly. Maybe I am missing something obvious, could you clarify?

ginsu added inline comments.Jul 13 2023, 10:15 AM

landing/competitor-feature-card.css
20 ↗	(On Diff #28601)	Yea definitely! Sorry I didn't make it clear the first time around. `CompetitorFeature` is a base component used in two places: `CompetitorFeatureCard` and `FeatureModal`. Initially when we had `min-height: 80px` in `CompetitorFeature` it made it so `FeatureModal` looked a bit strange: Because of this, we don't want `min-height: 80px` in the `FeatureModal`, but we still want it in `CompetitorFeatureCard` so that all the dividers in the cards are aligned regardless of length of the feature description: So I moved `min-height: 80px` to just the `CompetitorFeatureCard` and added the extra optional style prop in `CompetitorFeature` so that the `CompetitorFeatureCard` can still have the `min-height: 80px` but `FeatureModal` won't have it