-
Notifications
You must be signed in to change notification settings - Fork 0
DEV: automatic timezone detection + manual override support #1
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is sound, but I think the user pref page is not the best place, since the user will still have to remember to navigate to the preference page and save to update their timezone.
IMO a better way would be to store the current user timezone on page load, then whenever the user navigates between pages check if the timezone != moment.tz.guess()
and if it is then update their timezone. This would require some core change though instead of a theme component, unless you do this:
api.modifyClass("route:discourse", {
pluginId: "auto-timezone",
willTransition() {
super.willTransition(...arguments);
// user timezone switcheroo here
}
});
But then you still are doing modifyClass which isn't great...if you want I can help you do this as a POC then you can make a post on dev about it and see what people think?
return; | ||
} | ||
|
||
const browserTimezone = Intl.DateTimeFormat().resolvedOptions().timeZone; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just use moment.tz.guess()
here, it's what we use everywhere else
|
||
const browserTimezone = Intl.DateTimeFormat().resolvedOptions().timeZone; | ||
|
||
const lastDetectedTimezone = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should fallback to || currentUser.user_option?.timezone
to cover people that don't have this custom field
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I'm making the custom field. So are you saying currentUser.user_option?.timezone
just already exists? Then I can just use that instead, and no fallback?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes we already store the user's timezone. It's only set on login currently, or if they go change it manually in the user preferences
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought you were doinga custom field for "accountability" or something lol
currentUser.save(); | ||
} | ||
|
||
api.modifyClass("controller:preferences/interface", { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
David will kill you 😂 I also don't think this is the best place for this, since the user will still have to remember to navigate to the preference page and save to update their timezone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haha, david is already my arch nemesis these days. I don't get the problem though? What does that mean, "the user will still have to remember…"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like, you want this to happen transparently if the timezone changes right? If you have to go to the user preferences page for this to happen it defeats the purpose
I don't get it. Why would I do that every single time the user navigates? |
@chapoi because you want this to happen automatically right, you don't want the user to have to go to the preferences page and set their timezone manually? The browser has no way of notifying JS that the user's timezone has changed, so we need some way of comparing it against what we have their timezone stored as in the user options table anytime. The user navigates all the time, so this is a good place to do this check |
This commit adds registerUserProfileBeforeSaveCallback for the user profile, which passes in the profile model, so themes and components can add custom logic here and change properties. Done for discourse/automatic-timezone#1
where are we with this? @martin-brennan |
@CvX I think we can just close this for now |
Goal:
This component automatically set the timezone, based on the detected browser timezone.
If the timezone is manually changed by the user (not sure what the usecase would be, but I've added support just in case), it will remain this way until a new timezone has been detected (ie the user moved to a different location).