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

Chore: support setting timezone for cronjob #1664

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

Conversation

StrongMonkey
Copy link
Contributor

@StrongMonkey StrongMonkey commented Feb 6, 2025

#1480

Adding support to run cronjob in a specific timezone. It will default to server's own timezone if it is not set in API. The browser will now set the user's own timezone as default but user will be able to change it.

https://www.loom.com/share/052bd151e46b4c1cb37e28b281687a50

Comment on lines 123 to 122
<TimezoneSelection
label="Timezone"
onChange={(timezone) =>
form.setValue("timezone", timezone, {
shouldValidate: true,
shouldDirty: true,
})
}
value={form.watch("timezone")}
/>

Copy link
Contributor

Choose a reason for hiding this comment

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

can you use ControlledCustomInput here? it provides some useful accessibility features

Copy link
Contributor

Choose a reason for hiding this comment

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

It also provides a standard method for providing a label, so you can omit that from TimezoneSelection.tsx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ryanhopperlowe ok, I did some effort and have no idea how to use it outside a form... 🤷

maybe you can take a look after it. It is used in a different place where form is not provided.

@StrongMonkey
Copy link
Contributor Author

@ryanhopperlowe changes pushed

Copy link
Contributor

@ryanhopperlowe ryanhopperlowe left a comment

Choose a reason for hiding this comment

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

thank you!!

@StrongMonkey
Copy link
Contributor Author

Per Darren's UX suggestion:

We are going to change the UX so that user doesn't need to pick timezone in current UX. Browser will by default send its current timezone and backend will save it and use for cronjob. It will only display the timzeone in UI when browser's current timezone is different than the one set in database.

@StrongMonkey
Copy link
Contributor Author

I changed the UX to match what we have discussed for disabling selection of a timezone now.

https://www.loom.com/share/c12007f78e7f4219970e89056257505f

@ryanhopperlowe
Copy link
Contributor

The layout for the schedulers seems to be shifted depending on the existence of a timezone. Looks a bit wonky

@StrongMonkey
Copy link
Contributor Author

@ryanhopperlowe yeah I agree... don't have a a clear solution for that. It is a bit weird if you have one entry for the same timezone but one different one in a different timezone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants