cal.com
cal.com copied to clipboard
calcom
feat:toggleTheme
What does this PR do?
This pr adds a feature which allows the user the app theme. Fixes #9047 Hopefully this is the approach u guys want. https://www.loom.com/share/e61586f54e3140279c05597214db8068
How should this be tested?
- [x] Test A->changing the themes and checking the change in db
- [ ] Test B
@firefox292 is attempting to deploy a commit to the cal Team on Vercel.
A member of the Team first needs to authorize it.
![vercel[bot] avatar](https://avatars.githubusercontent.com/in/8329?v=4 "Published by a pub.dev verified publisher"){kind=small}
The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Comments | Updated (UTC) |
|---|---|---|---|---|
| ui | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Aug 12, 2023 6:06pm |
📦 Next.js Bundle Analysis for @calcom/web
This analysis was generated by the Next.js Bundle Analysis action. 🤖
One Page Changed Size
The following page changed size from the code in this PR compared to its base branch:
| Page | Size (compressed) | First Load | % of Budget (350 KB) |
|---|---|---|---|
/settings/my-account/general |
347.92 KB |
502.08 KB | 143.45% (🟡 +0.21%) |
Details
Only 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 next/link is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.
Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis
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 "+/-
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Thanks for the contribution @firfox341. It does look good! Left some comments regarding code to be removed.
Also, what would be the rationale behind using a new column in the database versus the existing
themecolumn?
hey @leog what i understood from the code was that 'theme' column is specific to public booking page so we need to store the two separately right, one for the complete website theme and one for the public booking page theme, correct if i am wrong pretty new to cal.com codebase.
Makes sense @firfox341. I would like to invoke the presence of @Jaibles for this one. I'm not sure we want to maintain both variables at the same time; if you choose dark mode why would you like your booking page to be light mode or automatic? Just in case, his blessing is always a good thing to have 😄
Makes sense @firfox341. I would like to invoke the presence of @Jaibles for this one. I'm not sure we want to maintain both variables at the same time; if you choose dark mode why would you like your booking page to be light mode or automatic? Just in case, his blessing is always a good thing to have 😄
Ya cool I didn't want mess with other things..Anyway we'll go with what @Jaibles says.
@leog @saurabbot is the question: "Should we allow people to have different dark/light mode settings for app.cal.com vs their public booking page"? If so, the answer is definitely yes.
I think a perfectly likely scenario is people who prefer dark mode wanting to always use that in the app, but wanting to respect the preference of their bookers and just keeping that to "System default".
@leog @saurabbot is the question: "Should we allow people to have different dark/light mode settings for app.cal.com vs their public booking page"? If so, the answer is definitely yes.
I think a perfectly likely scenario is people who prefer dark mode wanting that in the app, but wanting to go with system settings for the public page so that their bookers get to see what they prefer.
oh great then ....this should be fine i guess then.
We do need to use the new svgs though. I haven't had a chance to optimise them yet in terms of filesize.
gtg right ? is the yarn.lock change fine ? could you guys merge these prs also https://github.com/calcom/cal.com/pull/8975 https://github.com/calcom/cal.com/pull/8925
gtg right ?
@saurabbot We need to add the new svgs (attached below) and also, can we move these to General settings page since they are related to your own in-app experience? These settings don't affect the public booking page, so they should not go in the Appearance page .
Optimised svgs
gtg right ?
@saurabbot We need to add the new svgs (attached below) and also, can we move these to General settings page since they are related to your own in-app experience? These settings don't affect the public booking page, so they should not go in the Appearance page .
Optimised svgs
Cool thanks @Jaibles
@saurabbot any update on this PR?
Hey @Udit-takkar this was done long back and ready from my side, I was waiting for you guys to test the feature.
Hey @Udit-takkar this was done long back and ready from my side, I was waiting for you guys to test the feature.
Hey @saurabbot. Actually, we were waiting for you to resolve the comments we did in the code (addressed already) and request a re-review so it returns to us to review again to be able to approve it. Take that into account for future contributions, so we can merge great contributions like yours faster 👍
If you can resolve the conflicts, I think it should be good to go! Thanks again!
Hey @Udit-takkar this was done long back and ready from my side, I was waiting for you guys to test the feature.
Hey @saurabbot. Actually, we were waiting for you to resolve the comments we did in the code (addressed already) and request a re-review so it returns to us to review again to be able to approve it. Take that into account for future contributions, so we can merge great contributions like yours faster 👍
If you can resolve the conflicts, I think it should be good to go! Thanks again!
hey @leog my bad, ill fix the conflicts right away.
@leog gtg i guess..
@firefox292 did you tested the latest? There is some problem with loading the svgs and when saving, the toast message does not look good. Please test it locally again and tackle those issues.
@saurabbot are you able to continue working on this PR? we might need to close it if there is no more activity in it 😓
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 2 committers have signed the CLA.
:x: saurabbot
:x: firefox292
You have signed the CLA already but the status is still pending? Let us recheck it.
