fix: UI mismatch with form state #10651

MehulZR · 2023-08-08T22:25:26Z

What does this PR do?

Fixes #10647 (issue)
~~Fixes #10390 (issue)~~

video.mp4

Type of change

Bug fix (non-breaking change which fixes an issue)

Mandatory Tasks

Make sure you have self-reviewed the code. A decent size PR without self-review might be rejected.

vercel · 2023-08-08T22:25:31Z

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	Jan 23, 2024 3:48pm

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
calcom-web-canary	⬜️ Ignored (Inspect)			Jan 23, 2024 3:48pm

vercel · 2023-08-08T22:25:32Z

@MehulZR is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

github-actions · 2023-08-08T22:25:39Z

Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link.

apps/web/public/static/locales/en/common.json

github-actions · 2023-08-08T22:35:28Z

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

packages/features/form-builder/FormBuilder.tsx

hariombalhara · 2023-08-14T11:42:11Z

@MehulZR Could you please remove the changes related to #10390 from this PR as that's already solved(and being solved) elsewhere already.

Also, you should open separate PRs for separate fixes unless they are really really related. It makes it easy to review and merge the PR

Udit-takkar

@MehulZR please remove conflicts and changes related to #10390

MehulZR · 2023-08-14T20:05:41Z

While updating the PR, I noticed that it is possible to book a event with length of 0. This is possible when saving the form from a tab other than the Event Setup.

We can solve it in two ways.

Either not letting the form submit via RHF.
On Submit, checking the value and throwing an Error like we do when there are no multiple durations selected.

I prefer the 2nd approach because If we use the 1st approach The user will not get any indication of an error on tabs other than Event Setup. And it also looks more consistent.

If you would like the 2nd approach too. What Error message do you want to be shown?

CLAassistant · 2023-08-28T09:19:21Z

All committers have signed the CLA.

hariombalhara

Let's address merge conflicts and unrelated changes first.

packages/features/form-builder/FormBuilder.tsx

Udit-takkar

@MehulZR do not push yarn.lock unless you change package.json.

Use this command to undo the changes in yarn.lock

github-actions · 2023-10-13T00:16:23Z

This PR is being marked as stale due to inactivity.

Udit-takkar · 2023-10-13T07:13:48Z

@MehulZR can you fix the conflicts in this PR?

apps/web/components/eventtype/EventTypeSingleLayout.tsx

apps/web/components/eventtype/EventWebhooksTab.tsx

hariombalhara · 2023-12-08T04:53:47Z

apps/web/components/eventtype/EventAdvancedTab.tsx

@@ -203,7 +205,7 @@ export const EventAdvancedTab = ({ eventType, team }: Pick<EventTypeSetupProps,
      <RequiresConfirmationController
        eventType={eventType}
        seatsEnabled={seatsEnabled}
-        metadata={eventType.metadata}
+        metadata={formMethods.getValues("metadata")}


I don't like the idea of sometimes getting the value from eventType and sometimes from formMethods. It makes the code complex for little gains.

Can we not just use formMethods everywhere. We could maybe do eventType = formMethods.getValues() and then we can keep on accessing values from eventType.

Going with formMethods only is what I will prefer too, will look into it.

There are some cases when we need the original eventType prop alongside formMethods values.
For ex. having both the count of webhooks and other form values in the same component.

what we can do is instead of calling formMethods.getValues("some_property_name") everywhere, we just do formValues = formMethods.getValues() saving the execution calls and cleaning up things.
but formMethods.getValues isn't reactive (value change doesn't cause re-render & it doesn't subscribe to input changes), so for some cases we will still be using formMethods.watch().

I have tested a bit with using the formValues strategy, as expected it is not reliable. We should use formMethods.getValues("some_prop")

hariombalhara

@MehulZR I resolved the conflicts and left some more comments.

joeauyeung · 2024-01-19T20:19:26Z

@MehulZR just a heads up there's more conflicts

joeauyeung

Double checked everything and tested some fields. I fixed the noShowFeeEnabled since it was returning true even if Stripe was disabled.

I did notice one thing that's consistent in main as well but for managed event types when changing from a set availability and back to "member's default availability" the option doesn't change.

I'm ok approving this PR since that bug wasn't caused by the changes made and it's such a huge QOL improvement.

Changes have been incorporated

emrysal

Fixed merge conflicts, approved

github-actions bot added Low priority Created by Linear-GitHub Sync ⚡ Quick Wins A collection of quick wins/quick fixes that are less than 30 minutes of work 🐛 bug Something isn't working 🧹 Improvements Improvements to existing features. Mostly UX/UI labels Aug 8, 2023

MehulZR commented Aug 8, 2023

View reviewed changes

apps/web/public/static/locales/en/common.json Outdated Show resolved Hide resolved

MehulZR commented Aug 8, 2023

View reviewed changes

apps/web/public/static/locales/en/common.json Outdated Show resolved Hide resolved

vercel bot deployed to Preview – ui August 8, 2023 22:29 View deployment

MehulZR commented Aug 8, 2023

View reviewed changes

packages/features/form-builder/FormBuilder.tsx Outdated Show resolved Hide resolved

Udit-takkar reviewed Aug 14, 2023

View reviewed changes

MehulZR requested a review from Udit-takkar September 3, 2023 21:32

hariombalhara requested changes Sep 4, 2023

View reviewed changes

packages/features/form-builder/FormBuilder.tsx Outdated Show resolved Hide resolved

MehulZR force-pushed the fix_form branch from 025c068 to f214bd5 Compare September 4, 2023 10:54

vercel bot deployed to Preview – ui September 4, 2023 10:58 View deployment

MehulZR requested a review from hariombalhara September 4, 2023 11:00

Udit-takkar reviewed Sep 4, 2023

View reviewed changes

MehulZR force-pushed the fix_form branch from f214bd5 to bbc18f1 Compare September 4, 2023 11:57

vercel bot deployed to Preview – ui September 4, 2023 12:01 View deployment

MehulZR requested a review from Udit-takkar September 4, 2023 12:08

github-actions bot added the 🚨 merge conflict This PR has a merge conflict that has to be addressed label Sep 28, 2023

github-actions bot added the Stale label Oct 13, 2023

github-actions bot removed the Stale label Oct 14, 2023

Merge remote-tracking branch 'origin/main' into fix_form

f7b40be

hariombalhara reviewed Dec 8, 2023

View reviewed changes

apps/web/components/eventtype/EventTypeSingleLayout.tsx Outdated Show resolved Hide resolved

hariombalhara reviewed Dec 8, 2023

View reviewed changes

apps/web/components/eventtype/EventWebhooksTab.tsx Outdated Show resolved Hide resolved

hariombalhara reviewed Dec 8, 2023

View reviewed changes

hariombalhara previously requested changes Dec 8, 2023

View reviewed changes

Fix: Sync form state with ui for more fields & slight code refactor

2166edc

MehulZR requested a review from hariombalhara December 14, 2023 08:25

keithwillcode added this to the v3.7 milestone Jan 10, 2024

keithwillcode added the community Created by Linear-GitHub Sync label Jan 11, 2024

keithwillcode modified the milestones: v3.7, v3.8 Jan 15, 2024

MehulZR and others added 2 commits January 21, 2024 01:10

Merge branch 'main' into fix_form

fec5069

Merge branch 'main' into fix_form

42f8de6

github-actions bot removed the 🚨 merge conflict This PR has a merge conflict that has to be addressed label Jan 23, 2024

Merge branch 'main' into fix_form

f7b7729

emrysal added Medium priority Created by Linear-GitHub Sync and removed Low priority Created by Linear-GitHub Sync labels Jan 23, 2024

Ensure Stripe app is enabled to disable seats

bf63686

github-actions bot added the Low priority Created by Linear-GitHub Sync label Jan 23, 2024

Merge branch 'main' into fix_form

e07269f

joeauyeung previously approved these changes Jan 23, 2024

View reviewed changes

keithwillcode enabled auto-merge (squash) January 23, 2024 18:47

Merge branch 'main' into fix_form

8f10946

emrysal dismissed joeauyeung’s stale review via 8f10946 January 24, 2024 10:32

fix: isPending -> isLoading

5ae86e0

emrysal approved these changes Jan 24, 2024

View reviewed changes

keithwillcode merged commit 0a2e275 into calcom:main Jan 24, 2024
29 of 37 checks passed

MehulZR deleted the fix_form branch February 17, 2024 06:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: UI mismatch with form state #10651

fix: UI mismatch with form state #10651

MehulZR commented Aug 8, 2023 •

edited

Loading

vercel bot commented Aug 8, 2023 •

edited

Loading

vercel bot commented Aug 8, 2023

github-actions bot commented Aug 8, 2023 •

edited

Loading

github-actions bot commented Aug 8, 2023 •

edited

Loading

hariombalhara commented Aug 14, 2023

Udit-takkar left a comment

MehulZR commented Aug 14, 2023

CLAassistant commented Aug 28, 2023 •

edited

Loading

hariombalhara left a comment

Udit-takkar left a comment

github-actions bot commented Oct 13, 2023

Udit-takkar commented Oct 13, 2023

hariombalhara Dec 8, 2023 •

edited

Loading

MehulZR Dec 8, 2023

MehulZR Dec 9, 2023 •

edited

Loading

MehulZR Dec 9, 2023

hariombalhara left a comment

joeauyeung commented Jan 19, 2024

joeauyeung left a comment

emrysal left a comment

fix: UI mismatch with form state #10651

fix: UI mismatch with form state #10651

Conversation

MehulZR commented Aug 8, 2023 • edited Loading

What does this PR do?

Type of change

Mandatory Tasks

vercel bot commented Aug 8, 2023 • edited Loading

vercel bot commented Aug 8, 2023

github-actions bot commented Aug 8, 2023 • edited Loading

github-actions bot commented Aug 8, 2023 • edited Loading

📦 Next.js Bundle Analysis for @calcom/web

hariombalhara commented Aug 14, 2023

Udit-takkar left a comment

Choose a reason for hiding this comment

MehulZR commented Aug 14, 2023

CLAassistant commented Aug 28, 2023 • edited Loading

hariombalhara left a comment

Choose a reason for hiding this comment

Udit-takkar left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 13, 2023

Udit-takkar commented Oct 13, 2023

hariombalhara Dec 8, 2023 • edited Loading

Choose a reason for hiding this comment

MehulZR Dec 8, 2023

Choose a reason for hiding this comment

MehulZR Dec 9, 2023 • edited Loading

Choose a reason for hiding this comment

MehulZR Dec 9, 2023

Choose a reason for hiding this comment

hariombalhara left a comment

Choose a reason for hiding this comment

joeauyeung commented Jan 19, 2024

joeauyeung left a comment

Choose a reason for hiding this comment

emrysal left a comment

Choose a reason for hiding this comment

MehulZR commented Aug 8, 2023 •

edited

Loading

vercel bot commented Aug 8, 2023 •

edited

Loading

github-actions bot commented Aug 8, 2023 •

edited

Loading

github-actions bot commented Aug 8, 2023 •

edited

Loading

CLAassistant commented Aug 28, 2023 •

edited

Loading

hariombalhara Dec 8, 2023 •

edited

Loading

MehulZR Dec 9, 2023 •

edited

Loading