CAL-1673: Use dark/light colors from user profile settings in booker #8905
Conversation
CAL-1673 new booker: only dark mode for me
seems like light mode is not working, its always dark for me |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
||
| const PoweredBy = dynamic(() => import("@calcom/ee/components/PoweredBy")); | ||
| const DatePicker = dynamic(() => import("./components/DatePicker").then((mod) => mod.DatePicker), { | ||
| ssr: false, | ||
| }); | ||
|
|
||
| const useBrandColors = ({ brandColor, darkBrandColor }: { brandColor?: string; darkBrandColor?: string }) => { |
There was a problem hiding this comment.
Moved it to a separate file so booker component stays more readable <3
📦 Next.js Bundle Analysis for @calcom/webThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
Current Playwright Test Results Summary✅ 98 Passing - Run may still be in progress, this comment will be updated as current testing workflow or job completes... (Last updated on 05/18/2023 06:16:13am UTC) Run DetailsRunning Workflow PR Update on Github Actions Commit: 7251874 Started: 05/18/2023 06:05:23am UTC
|
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Managed Event Types tests Can create managed event type
Retry 2 • Retry 1 • Initial Attempt |
1.48% (4)4 / 271 runsfailed over last 7 days |
25.46% (69)69 / 271 runsflaked over last 7 days |
📄 apps/web/playwright/embed-code-generator.e2e.ts • 1 Flake
Test Case Results
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Embed Code Generator Tests Event Type Edit Page open Embed Dialog for the Event Type
Retry 1 • Initial Attempt |
6.14% (17)17 / 277 runsfailed over last 7 days |
29.96% (83)83 / 277 runsflaked over last 7 days |
📄 apps/web/playwright/booking-seats.e2e.ts • 2 Flakes
Top 1 Common Error Messages
|
|
2 Test Cases Affected |
Test Case Results
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Booking with Seats -- new-booker Reschedule for booking with seats -- old-booker Should reschedule booking with seats and if everyone rescheduled it should be deleted
Retry 1 • Initial Attempt |
0% (0)0 / 276 runsfailed over last 7 days |
83.33% (230)230 / 276 runsflaked over last 7 days |
|
Booking with Seats -- old-booker Reschedule for booking with seats -- old-booker Should reschedule booking with seats and if everyone rescheduled it should be deleted
Retry 1 • Initial Attempt |
0% (0)0 / 276 runsfailed over last 7 days |
20.29% (56)56 / 276 runsflaked over last 7 days |
| useEffect(() => { | ||
| if (theme) setTheme(theme); | ||
| }, [setTheme, theme]); | ||
| }; |
There was a problem hiding this comment.
Is this util maybe worth having it in a more generic path to be applied somewhere else if makes sense? perhaps @calcom/lib?
There was a problem hiding this comment.
Yeah great question. I took the logic from the current booker, and actually not really sure if it's used anywhere else (and the current booker will be gone soon).
@sean-brydon what do you think about this? It pretty much combines most of your hooks into one hook – so perhaps it indeed makes sense to reuse it. On top of that I also added an effect that triggers the setTheme again, because in our case the theme is undefined during load, which would cause to set the default and not update it anymore later.
| @@ -113,6 +115,7 @@ export const getPublicEvent = async (username: string, eventSlug: string, prisma | |||
| image: `${WEBAPP_URL}/${users[0].username}/avatar.png`, | |||
| brandColor: users[0].brandColor, | |||
| darkBrandColor: users[0].darkBrandColor, | |||
| theme: users[0].theme, | |||
There was a problem hiding this comment.
It doesn't seem to use system theme for dynamic booking. I am not sure which is better, but currently dynamic group bookings always use system theme(by setting theme to null) as it's not clear whose theme to use, so we use system theme which is a good default.
There was a problem hiding this comment.
Also, I don't see handling for team.theme. That got added not too long ago.
The team handling is there, nevermind. Beautifully handled. Only, dynamic group booking handling is different.
There was a problem hiding this comment.
Good point! I'll change dynamic theme to system theme indeed. Makes a lot of sense! Thanks for spotting this!
|
@hariombalhara Thanks a lot for checking this! Just now updated the dynamic booking to indeed use the system theme. Makes a lot more sense. Ready to approve? 😁 |
What does this PR do?
New booker didn't use the theme setting in appearance to make the booker dark / light. With this PR that is now the case.
Fixes CAL-1673
Environment: Staging(main branch)
Type of change
How should this be tested?