Add timezone client-hints by rajeshg · Pull Request #303 · epicweb-dev/epic-stack

Add timezone client-hints by rajeshg · Pull Request #303 · epicweb-dev/epic-stack

Full credits to @kiliman for this work. https://github.com/kiliman/epic-stack-time-zone

Client hints for timezone are a nice add. The approach is similar to the way themes are handled currently. CH-time-zone will be added to the cookies.

Test Plan

Checklist

Tests updated
Docs updated

Screenshots

Choose a reason for hiding this comment

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

I love this. Just a few changes.

app/root.tsx Outdated Show resolved Hide resolved

app/root.tsx Outdated Show resolved Hide resolved

Comment on lines +22 to +24

		transform(value: string \| null) {
		return value ?? 'UTC'
		},

Choose a reason for hiding this comment

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

In this case, we don't need to transform the value at all and the fallback is already defined. I think we should not require the transform function. So let's remove it here.

	transform(value: string \| null) {
	return value ?? 'UTC'
	},

This change may necessitate a small update in the code below.

		[name in ClientHintNames]: ReturnType<
		(typeof clientHints)[name]['transform']
		>
		[name in ClientHintNames]: string

Choose a reason for hiding this comment

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

We lose some type-checking here. But couldn't think of a better way to do it.

Choose a reason for hiding this comment

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

Ah yeah, we'll need to figure out how to have our cake and eat it too here 😅 If you can't work it out, then that's fine. I'll come back to this later when I find time. Thanks!

Choose a reason for hiding this comment

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

Couldn't figure out :(

Why was this PR closed? I'd still like to have this feature eventually and I'd prefer you get git credit for your work :)

I might recommend you make any future PRs from a non-main branch though.

My mistake. You are right, I should have done that from a non-main branch. I attempted to copy over the latest changes from epicweb-dev/epic-stack to rajeshg/epic-stack.

Couldn't reopen this myself, since I lack the rights to reopen pull requests on this repo. I will submit a separate pull request, when I get a chance.

I can't reopen it either because you created it from main and now your main branch is different. This is another reason using a different branch helps 😅 Thanks!