fix: styles for the schedule component, make it mobile-friendly#15178
fix: styles for the schedule component, make it mobile-friendly#15178helgastogova merged 7 commits intomainfrom
Conversation
|
|
|
Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link. |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (05/23/24)1 reviewer was added to this PR based on Keith Williams's automation. "Add platform team as reviewer" took an action on this PR • (05/23/24)1 reviewer was added to this PR based on Keith Williams's automation. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Ignored Deployments
|
Current Playwright Test Results Summary✅ 321 Passing - ❌ 1 Failing - Run may still be in progress, this comment will be updated as current testing workflow or job completes... (Last updated on 05/23/2024 04:37:41pm UTC) Run DetailsRunning Workflow PR Update on Github Actions Commit: 071fbda Started: 05/23/2024 04:28:55pm UTC ❌ Failures📄 apps/web/playwright/teams.e2e.ts • 1 FailureTest Case Results
|
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Managed Event Types Can create managed event type
Retry 1 • Initial Attempt |
6.93% (16)16 / 231 runsfailed over last 7 days |
3.46% (8)8 / 231 runsflaked over last 7 days |
📄 apps/web/playwright/reschedule.e2e.ts • 1 Flake
Test Case Results
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Reschedule Tests Opt in event should be PENDING when rescheduled by USER
Retry 1 • Initial Attempt |
0% (0)0 / 259 runsfailed over last 7 days |
0.39% (1)1 / 259 runflaked over last 7 days |
📄 packages/embeds/embed-core/playwright/tests/preview.e2e.ts • 1 Flake
Test Case Results
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Preview Preview - embed-core should load if correct embedLibUrl is provided
Retry 1 • Initial Attempt |
0% (0)0 / 225 runsfailed over last 7 days |
32% (72)72 / 225 runsflaked over last 7 days |
📄 apps/web/playwright/login.e2e.ts • 1 Flake
Test Case Results
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
user can login & logout succesfully -- future login flow user & logout using dashboard
Retry 2 • Retry 1 • Initial Attempt |
2.64% (6)6 / 227 runsfailed over last 7 days |
30.84% (70)70 / 227 runsflaked over last 7 days |
📄 apps/web/playwright/profile.e2e.ts • 1 Flake
Test Case Results
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Update Profile Can update a users email (verification enabled)
Retry 1 • Initial Attempt |
29.25% (74)74 / 253 runsfailed over last 7 days |
34.39% (87)87 / 253 runsflaked over last 7 days |
📄 packages/app-store/routing-forms/playwright/tests/basic.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 |
|---|---|---|
|
Routing Forms Seeded Routing Form Router URL should work
Retry 1 • Initial Attempt |
0.44% (1)1 / 228 runfailed over last 7 days |
13.16% (30)30 / 228 runsflaked over last 7 days |
|
Routing Forms Seeded Routing Form Test preview should return correct route
Retry 1 • Initial Attempt |
0.44% (1)1 / 227 runfailed over last 7 days |
33.92% (77)77 / 227 runsflaked over last 7 days |
📄 apps/web/playwright/teams.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 |
|---|---|---|
|
Teams - NonOrg -- future Can create a booking for Round Robin EventType
Retry 1 • Initial Attempt |
7.95% (19)19 / 239 runsfailed over last 7 days |
24.69% (59)59 / 239 runsflaked over last 7 days |
|
Teams - NonOrg -- legacy Can create a booking for Round Robin EventType
Retry 1 • Initial Attempt |
7.05% (17)17 / 241 runsfailed over last 7 days |
28.22% (68)68 / 241 runsflaked over last 7 days |
📄 apps/web/playwright/organization/booking.e2e.ts • 1 Flake
Test Case Results
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Bookings Team Event Can create a booking for Round Robin EventType
Retry 1 • Initial Attempt |
6.93% (16)16 / 231 runsfailed over last 7 days |
28.14% (65)65 / 231 runsflaked over last 7 days |
dosubot
Bot
added
ui
| {!!watchDayRange.length && !disabled && <div className="block">{CopyButton}</div>} | ||
| </div> | ||
| <> | ||
| {watchDayRange.length > 0 && ( |
There was a problem hiding this comment.
How about we move this watchDayRange.length > 0 check one level higher. So instead of adding this conditional watchDayRange.length > 0 inside of the Day ranges component we remove that check completely and maybe do something like this: !!watchDayRange ? Day ranges component : Skeleton component
There was a problem hiding this comment.
The !! will convert the watchDayRange variable to its boolean equivalent, similar to what watchDayRange.length > 0 does. Or you can use Boolean(watchDayRange) but thats just a personal preference since both of them are the same
There was a problem hiding this comment.
Hey @Ryukemeister, yes, I agree with you, it really does seem like it needs to be written exactly like this.
{!!watchDayRange ? (
<div className="flex sm:gap-2">
<DayRanges
userTimeFormat={userTimeFormat}
labels={labels}
control={control}
name={name}
disabled={disabled}
className={{
dayRanges: className?.dayRanges,
timeRangeField: className?.timeRangeField,
}}
/>
{!!watchDayRange.length && !disabled && <div className="block">{CopyButton}</div>}
</div>
) : (
<SkeletonText className="ml-1 mt-2.5 h-6 w-48" />
)}
but if you try to do that, you will see that in that case, we have one extra empty div
the same will be if I use !!watchDayRange instead of watchDayRange.length > 0
The difference between these checks in this case:
- watchDayRange.length > 0
This line checks if the length of the watchDayRange array is greater than zero. This means there must be at least one element in the array for the condition to return true. - !!watchDayRange
This line first converts watchDayRange to a boolean value. If watchDayRange is an object or array (even if empty), !!watchDayRange will always return true because objects and arrays are "truthy" values in JavaScript. Thus, this condition checks for the existence of the array, not its emptiness.
The difference is that the first condition checks for the presence of elements within the array, while the second checks for the existence of the array itself, regardless of its content. If watchDayRange is an empty array ([]), the first condition will return false, while the second will return true.
And if I do:
{watchDayRange.length > 0 ? (
<div className="flex sm:gap-2">
<DayRanges
userTimeFormat={userTimeFormat}
labels={labels}
control={control}
name={name}
disabled={disabled}
className={{
dayRanges: className?.dayRanges,
timeRangeField: className?.timeRangeField,
}}
/>
{!!watchDayRange.length && !disabled && <div className="block">{CopyButton}</div>}
</div>
) : (
<SkeletonText className="ml-1 mt-2.5 h-6 w-48" />
)}
I will see a skeleton when I don't need to see it
so, we need to check if
- {watchDayRange ? ( ... ) : ( ... )}:
This checks if watchDayRange is defined. If watchDayRange is neither null nor undefined, the code inside the first part of the condition (after ?) is executed. If watchDayRange is null or undefined, is displayed. - {watchDayRange.length > 0 && ( ... )}:
This checks if the watchDayRange array contains any elements. If the array is not empty (length > 0), it renders a block containing the DayRanges component.
There was a problem hiding this comment.
What if we do watchDayRange && watchDayRange.length > 0? That way day ranges only renders when watch day range is defined and its not empty. So what about watchDayRange && watchDayRange.length > 0 ? < DayRanges/> : < Skeleton/>
There was a problem hiding this comment.
watchDayRange && watchDayRange.length > 0
will give us false if we have [] and we will see a <Skeleton /> when we don't want to see it
so, code might look like that
<>
{!watchDayRange && <SkeletonText className="ml-1 mt-2.5 h-6 w-48" />}
{watchDayRange.length > 0 && (
<div className="flex sm:gap-2">
<DayRanges
userTimeFormat={userTimeFormat}
labels={labels}
control={control}
name={name}
disabled={disabled}
className={{
dayRanges: className?.dayRanges,
timeRangeField: className?.timeRangeField,
}}
/>
{!!watchDayRange.length && !disabled && <div className="block">{CopyButton}</div>}
</div>
)}
</>
or like that
<>
{watchDayRange ? (
<>
{watchDayRange.length > 0 && (
<div className="flex sm:gap-2">
<DayRanges
userTimeFormat={userTimeFormat}
labels={labels}
control={control}
name={name}
disabled={disabled}
className={{
dayRanges: className?.dayRanges,
timeRangeField: className?.timeRangeField,
}}
/>
{!!watchDayRange.length && !disabled && <div className="block">{CopyButton}</div>}
</div>
)}
</>
) : (
<SkeletonText className="ml-1 mt-2.5 h-6 w-48" />
)}
</>
but not like that
<>
{watchDayRange && watchDayRange.length > 0 ? (
<div className="flex sm:gap-2">
<DayRanges
userTimeFormat={userTimeFormat}
labels={labels}
control={control}
name={name}
disabled={disabled}
className={{
dayRanges: className?.dayRanges,
timeRangeField: className?.timeRangeField,
}}
/>
{!!watchDayRange.length && !disabled && <div className="block">{CopyButton}</div>}
</div>
) : (
<SkeletonText className="ml-1 mt-2.5 h-6 w-48" />
)}
</>
I will update it for the first version
sean-brydon
left a comment
sean-brydon
left a comment
There was a problem hiding this comment.
I'm happy with this thanks @helgastogova :)
@Ryukemeister - I'll wait for your approval before we merge as you left some previous comments.
|
wow this sick! |
…om#15178) * fix styles for the schedule component, make it mobile-friendly * CR changes * no need that condition
6 participants
What does this PR do?
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Visually check the Schedule component view on the /availability/ page.
before (wide screen)
before (mobile view)
after (wide screen)
after (mobile view)
design check (compare with Figma)
