12h default time format #858
Conversation
|
|
||
| # revision identifiers, used by Alembic. | ||
| revision = '330fdd8cd0f8' | ||
| down_revision = '16c0299eff23' |
There was a problem hiding this comment.
This is the last revision id from #854, so when both are merged, there shouldn't be conflicts about branching revision numbers.
| (user?.settings?.colourScheme === 'dark' | ||
| || (user?.settings?.colourScheme === 'system' && browserPrefersDark) | ||
| || (!user?.settings?.colourScheme && browserPrefersDark)) | ||
| (user.settings?.colourScheme === 'dark' | ||
| || (user.settings?.colourScheme === 'system' && browserPrefersDark) | ||
| || (!user.settings?.colourScheme && browserPrefersDark)) |
There was a problem hiding this comment.
Unfortunately we can't use TS at this place (or do we?), so we can't imply types here. But the user object is always to be expected, so I removed those first question marks.
There was a problem hiding this comment.
imo, we should remove this and just default to system.
There was a problem hiding this comment.
Agreed, I can remove that.
There was a problem hiding this comment.
Oh wait, actually we currently need that to apply the user settings on a fresh page reload. Also this ensures no "flashing" if the user setting differs from the system theme. I would leave it like this for now.
| const format = Number(user?.setttings?.timeFormat ?? is12HourTime); | ||
| const user = JSON.parse(localStorage?.getItem('tba/user') ?? '{}') as User; | ||
| const detected = Intl.DateTimeFormat().resolvedOptions().hour12 ? 12 : 24; | ||
| const format = Number(user.settings?.timeFormat ?? detected); |
There was a problem hiding this comment.
The actual fix. A typo. What else. 😅
There was a problem hiding this comment.
Good find @devmount! \o/
Pulled the branch and tried it out, works great for me locally.
There is one frontend linting warning (it was attached automatically to your PR) or you can also see it here.
Thanks!
|
Thanks Rob!
Yes I saw that, the App.vue contains a function |
| (user?.settings?.colourScheme === 'dark' | ||
| || (user?.settings?.colourScheme === 'system' && browserPrefersDark) | ||
| || (!user?.settings?.colourScheme && browserPrefersDark)) | ||
| (user.settings?.colourScheme === 'dark' | ||
| || (user.settings?.colourScheme === 'system' && browserPrefersDark) | ||
| || (!user.settings?.colourScheme && browserPrefersDark)) |
There was a problem hiding this comment.
imo, we should remove this and just default to system.
Description of the Change
This PR fixes a bug, were the user setting for the time format didn't apply correctly. Also, h12 is set now as a new default.
Tip
I already took PR #854 into account for db migrations, although it is not merged yet.
Applicable Issues
Closes #850