Skip to content

fix: app store spacing issues#6364

Closed
G3root wants to merge 24 commits intomainfrom
6336-cal-710-app-store-spacing-issues-check-all-margins-against-design
Closed

fix: app store spacing issues#6364
G3root wants to merge 24 commits intomainfrom
6336-cal-710-app-store-spacing-issues-check-all-margins-against-design

Conversation

@G3root
Copy link
Copy Markdown
Contributor

@G3root G3root commented Jan 10, 2023

What does this PR do?

Fixes #6336

Environment: Staging(main branch) / Production

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How should this be tested?

  • Test A
  • Test B

Checklist

  • I haven't read the contributing guide
  • My code doesn't follow the style guidelines of this project
  • I haven't performed a self-review of my own code and corrected any misspellings
  • I haven't commented my code, particularly in hard-to-understand areas
  • I haven't checked if my PR needs changes to the documentation
  • I haven't checked if my changes generate no new warnings
  • I haven't added tests that prove my fix is effective or that my feature works
  • I haven't checked if new and existing unit tests pass locally with my changes

@G3root G3root requested a review from a team January 10, 2023 11:54
@G3root G3root linked an issue Jan 10, 2023 that may be closed by this pull request
[CAL-710] /App store : Spacing issues - Check all margins against design #6336
Closed
@linear
Copy link
Copy Markdown

linear bot commented Jan 10, 2023

CAL-710 /App store : Spacing issues - Check all margins against design

  • I have clearly spec'd all spacing in designs. Check all margin & spacing
  • Also check the styles of the headers.

CleanShot 2023-01-08 at 12.41.02@2x.png

View in Figma

@vercel
Copy link
Copy Markdown

vercel bot commented Jan 10, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
cal ❌ Failed (Inspect) Jan 14, 2023 at 3:48PM (UTC)

@PeerRich
Copy link
Copy Markdown
Member

PeerRich commented Jan 10, 2023

@Jaibles can you test this preview, also on mobile and tablet?

@ciaranha
Copy link
Copy Markdown
Member

ciaranha commented Jan 10, 2023 •
edited
Loading

Looks much better! Will add the small remaining issues here:

Desktop

Tablet

CleanShot 2023-01-10 at 14 59 30@2x

Mobile

@PeerRich
Copy link
Copy Markdown
Member

PeerRich commented Jan 10, 2023

oh yeah mobile looks odd, how should it be? @Jaibles

I don't mind having it done in this PR

@ciaranha
Copy link
Copy Markdown
Member

ciaranha commented Jan 10, 2023

@PeerRich @G3root mobile designs here.

G3root
G3root commented Jan 11, 2023
View reviewed changes
packages/features/shell/Shell.tsx
title?: string;
heading?: ReactNode;
subtitle?: ReactNode;
headerClassName?: string;
Copy link
Copy Markdown
Contributor Author

@G3root G3root Jan 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a workaround to hide title and subtitle in app pages on tablet

@G3root
Copy link
Copy Markdown
Contributor Author

G3root commented Jan 11, 2023

new changes

  • fixed Missing the 12px margin below subheaders
    Screenshot 2023-01-11 at 22-48-10 App Store Cal com

  • slider button hover state
    Screenshot 2023-01-11 22:52:17

  • category card
    Screenshot 2023-01-11 at 22-53-34 App Store Cal com

  • mobile and tablet view

@G3root
Copy link
Copy Markdown
Contributor Author

G3root commented Jan 11, 2023

@Jaibles pushed some changes, can you check the preview

@G3root G3root requested a review from PeerRich January 13, 2023 12:09
G3root and others added 6 commits January 14, 2023 21:11
@G3root @PeerRich
fix: HOSTED_CAL_FEATURES env doesn't get passed to the frontend (#6392
cd2dab9 
)

* fix: rename HOSTED_CAL_FEATURES into NEXT_PUBLIC_HOSTED_CAL_FEATURES

* chore: add the new env to example

* fix: docs link

Co-authored-by: Peer Richelsen <peeroke@gmail.com>
@Udit-takkar @PeerRich @G3root
fix: remove website url in download recording (#6466)
fa1da76 
Co-authored-by: Peer Richelsen <peeroke@gmail.com>
@joeauyeung @G3root
Disable email input if new user invited to team (#6471)
e307cbd
@phishy @PeerRich
@zomars @G3root
disables signups via env variable (#6212)
b76408b 
* disables signups via env variable

* Apply suggestions from code review

Co-authored-by: Peer Richelsen <peeroke@gmail.com>
Co-authored-by: Omar López <zomars@me.com>
@CarinaWolli @G3root
Reusable Upgrade component for teams only features (#6473)
a7e5717 
Co-authored-by: CarinaWolli <wollencarina@gmail.com>
@PeerRich @G3root
i18n udpate
f8c9fa5
@github-actions github-actions bot added the ❗️ .env changes contains changes to env variables label Jan 14, 2023
@G3root G3root marked this pull request as draft January 14, 2023 15:45
@G3root
Copy link
Copy Markdown
Contributor Author

G3root commented Jan 14, 2023

@PeerRich marking this PR as draft because some other commits are added to this PR , similar to what happened with @sean-brydon . i'll make another PR with a clean branch.

@G3root G3root closed this Jan 14, 2023
@G3root G3root mentioned this pull request Jan 14, 2023
Merged
7 tasks
@G3root G3root deleted the 6336-cal-710-app-store-spacing-issues-check-all-margins-against-design branch January 14, 2023 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PeerRich PeerRich Awaiting requested review from PeerRich

Assignees

No one assigned

Labels

❗️ .env changes contains changes to env variables

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[CAL-710] /App store : Spacing issues - Check all margins against design

7 participants

@G3root @PeerRich @ciaranha @Udit-takkar @joeauyeung @phishy @CarinaWolli