Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Timezone in Email Embed not working #15525 #15537

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

acharyayush
Copy link

What does this PR do?

Timezone selection in email embed page was disabled. I enabled it by updating the timezone of localstorage when user selects another timezone

##Before
https://www.loom.com/share/c5a5e587307241ecb76fd349ca33551b

##After
https://www.loom.com/share/a017ffb10f5641ad9d48130e3c964883

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected)
  • I have added a Docs issue here if this PR makes changes that would require a documentation change
  • I have added or modified automated tests that prove my fix is effective or that my feature works (PRs might be rejected if logical changes are not properly tested)

Copy link

vercel bot commented Jun 22, 2024

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

A member of the Team first needs to authorize it.

@CLAassistant
Copy link

CLAassistant commented Jun 22, 2024

CLA assistant check
All committers have signed the CLA.

@graphite-app graphite-app bot requested a review from a team June 22, 2024 17:14
@graphite-app graphite-app bot added the community Created by Linear-GitHub Sync label Jun 22, 2024
@dosubot dosubot bot added embed area: embed, widget, react embed ui area: UI, frontend, button, form, input 🐛 bug Something isn't working labels Jun 22, 2024
Copy link

graphite-app bot commented Jun 22, 2024

Graphite Automations

"Add community label" took an action on this PR • (06/22/24)

1 label was added to this PR based on Keith Williams's automation.

"Add consumer team as reviewer" took an action on this PR • (06/22/24)

1 reviewer was added to this PR based on Keith Williams's automation.

@@ -299,7 +299,7 @@ const EmailEmbed = ({
<Collapsible open>
<CollapsibleContent>
<div className="text-default mb-[9px] text-sm">{t("timezone")}</div>
<TimezoneSelect id="timezone" value={timezone} isDisabled />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@acharyayush how about toggling the value of the prop isDisabled?
<TimezoneSelect id="timezone" value={timezone} isDisabled={false} />

Toggled the value of isDisabled to false instead of removing
@@ -64,6 +66,7 @@ export function TimezoneSelectComponent({
...(data ? addCitiesToDropdown(data) : {}),
...addCitiesToDropdown(cities),
}}
onChange={({ value }) => setTimezone(value)}
Copy link
Contributor

@Ryukemeister Ryukemeister Jun 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't onChange and onInputChange handlers one and the same? The issue was that the timezone selector was disabled because of previously setting the isDisabled prop to true right? So is this really needed now?

@@ -299,7 +299,7 @@ const EmailEmbed = ({
<Collapsible open>
<CollapsibleContent>
<div className="text-default mb-[9px] text-sm">{t("timezone")}</div>
<TimezoneSelect id="timezone" value={timezone} isDisabled />
<TimezoneSelect id="timezone" value={timezone} isDisabled={false} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hariombalhara was this disabled on purpose?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Pradumn27 worked on this feature. I believe there has to be a reason. It isn't accidental.

Copy link
Contributor

@Udit-takkar Udit-takkar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If i click on "See available times" with a different time zone selected in dropdown it still opens the booking page in my default timezone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working community Created by Linear-GitHub Sync embed area: embed, widget, react embed ui area: UI, frontend, button, form, input
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants