fix: component styles and sign up / onboarding dark mode #12581
Conversation
|
Someone is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
|
|
|
Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link to collect XP and win prizes! |
stabildev
changed the title
📦 Next.js Bundle Analysis for @calcom/webThis analysis was generated by the Next.js Bundle Analysis action. 🤖 Sixty-eight Pages Changed SizeThe following pages changed size from the code in this PR compared to its base branch:
DetailsOnly the gzipped size is provided here based on an expert tip. First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If Any third party scripts you have added directly to your app using the The "Budget %" column shows what percentage of your performance budget the First Load total takes up. For example, if your budget was 100kb, and a given page's first load size was 10kb, it would be 10% of your budget. You can also see how much this has increased or decreased compared to the base branch of your PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. If you see "+/- <0.01%" it means that there was a change in bundle size, but it is a trivial enough amount that it can be ignored. |
| "bg-brand-default hover:bg-brand-emphasis focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-offset focus-visible:ring-brand-default text-brand disabled:bg-brand-subtle disabled:text-brand-subtle disabled:opacity-40 disabled:hover:bg-brand-subtle disabled:hover:text-brand-default disabled:hover:opacity-40 dark:bg-white dark:text-black", | ||
| secondary: |
There was a problem hiding this comment.
These values should be correct already - without the dark: prefix
If they were not working in a specific area of the app (onboarding) the variables may not have been set
There was a problem hiding this comment.
Wow this is a great addition!
Dark mode looks awesome well done. I have one ask that we refrain from using dark: anywhere possible.
We have tokens defined in cal.com/apps/web/styles/globals.css
If they are not working on a specific page that is another issue. The rest of the app functions fine in darkmode.
Great PR thanks bud!
Edit: Looking at this more i think the issue you are seeing here is the cal brand colour is not set yet - due to that being a user defined thing in settings.
We will need to set them in classNames
Light Mode:
"--cal-brand": "#111827",
"--cal-brand-emphasis": "#101010",
"--cal-brand-text": "white",
Dark Mode:
--cal-brand: white;
--cal-brand-emphasis: #e1e1e1;
--cal-brand-text: black;
| @@ -106,16 +105,8 @@ const OnboardingPage = () => { | |||
|
|
|||
| return ( | |||
| <div | |||
| className="dark:bg-brand dark:text-brand-contrast text-emphasis min-h-screen" | |||
| className="dark:bg-brand dark:text-brand-contrast text-emphasis min-h-screen [--cal-brand-emphasis:#101010] [--cal-brand-subtle:9CA3AF] [--cal-brand-text:#FFFFFF] [--cal-brand:#111827] dark:[--cal-brand-emphasis:#e1e1e1] dark:[--cal-brand-text:#000000] dark:[--cal-brand:white]" | |||
There was a problem hiding this comment.
@stabildev - I pushed these changes to your branch to allow us to have the correct styling for both light and dark mode
(We only ever had the light mode vars - as thats all we supported)
apps/web/pages/signup.tsx
Outdated
| @@ -355,7 +355,10 @@ export default function Signup({ | |||
| router.push(GOOGLE_AUTH_URL); | |||
| }}> | |||
| <img | |||
| className={classNames("text-emphasis mr-2 h-5 w-5", premiumUsername && "opacity-50")} | |||
| className={classNames( | |||
| "text-emphasis mr-2 h-5 w-5 dark:invert", | |||
There was a problem hiding this comment.
Lets invert the google icon here too
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
Thanks!
I copied that approach from /login.tsx (changed over there as well now). Can confirm that everything looks as intended. With one exception:
IMO bg-transparent would make more sense on the secondary (outline) button:
|
|
Originally refactored the sign up page using AuthContainer like this
At the same time that 8dabbbd (signup refactor) happened |
Thanks for the feedback - lets stick with the designs that are in figma atm for the secondary button. I will discuss this with our designers to ensure this is the correct approach that we wish to take. For now i think leaving it as the |
|
All looks good to me @stabildev - great first contribution! I will try and figure out why linter is failing after these calls 🙏 |
Thanks @sean-brydon . There seems to be a mismatch with prettier and eslint regarding tailwind class sorting (especially those [custom_selector_classes]. |
Yeah i know! never seen it before and can't get it to pass either :') |
…arkmode' into components-and-darkmode
PeerRich
added
Medium priority
* fix divider border for addOnLeading * fix primary button in dark mode and password input border * signup dark mode and corner fix * onboarding dark mode * fix css var issue and use inline vars for light and dark mode * Invert google icon on dark mode * Fix typo * fix eslint errors with yarn lint:fix * use css vars on login page as well * running lint manually * Fix subtle * Fix * Fix * linting * linting * chore: restore main yarn.lock * fix: lint error --------- Co-authored-by: Peer Richelsen <peeroke@gmail.com> Co-authored-by: Sean Brydon <sean@brydon.io> Co-authored-by: sean-brydon <55134778+sean-brydon@users.noreply.github.com> Co-authored-by: sean-brydon <sean@cal.com> Co-authored-by: Alex van Andel <me@alexvanandel.com>
@sean-brydon I have the same issue in another project now: custom tailwind classes get sorted differently in prettier vs in eslint. Did you manage to fix this issue? |
* fix divider border for addOnLeading * fix primary button in dark mode and password input border * signup dark mode and corner fix * onboarding dark mode * fix css var issue and use inline vars for light and dark mode * Invert google icon on dark mode * Fix typo * fix eslint errors with yarn lint:fix * use css vars on login page as well * running lint manually * Fix subtle * Fix * Fix * linting * linting * chore: restore main yarn.lock * fix: lint error --------- Co-authored-by: Peer Richelsen <peeroke@gmail.com> Co-authored-by: Sean Brydon <sean@brydon.io> Co-authored-by: sean-brydon <55134778+sean-brydon@users.noreply.github.com> Co-authored-by: sean-brydon <sean@cal.com> Co-authored-by: Alex van Andel <me@alexvanandel.com>
Hi all, this PR fixes some minor issues I encountered when setting up Cal.com locally. Also added dark mode support to the onboarding.
What does this PR do?
1. Fix missing vertical divider with
addOnLeading:Before:
After (now congruent with
addOnSuffix:2. Fix
PasswordFieldborder getting brighter (in dark mode) when focused but not hoveredThis issue is related to 4438ca2 . Back then the
PasswordFieldinput was wrapped with a<div className="absolute">and some custom selectors. This wrapping<div>is not needed anymore because the show/hide password eye icon is now added to the input via theaddOnSuffixprop. Removing this wrapper resolves the issue.Before:
After:
3. Fix corners in new sign up form
Before:
After:
4. Fix primary button color in dark mode
In most places this was already applied manually. This PR adds the dark mode styling to the component itself for design consistency.
Before:
After:
5. Enable dark mode for the (new) sign up form and the complete onboarding process
Sign Up Form:
Getting Started:
6. Fix styling of Apple Calendar Setup
Before:
After:
After (Light mode):
Requirement/Documentation
Type of change
How should this be tested?
Visual check of the affected routes (in system dark mode), especially:
Mandatory Tasks
Checklist