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

Added ability to change FullCalendar locale #491

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

Conversation

jaspwr
Copy link

@jaspwr jaspwr commented Aug 22, 2023

Adds a setting allowing users to change FullCalendar locale and autodetects the users locale for the default setting.
Fixes #355.

@jaspwr jaspwr changed the title Added ability to change FullCalender locale Added ability to change FullCalendar locale Aug 22, 2023
Copy link
Collaborator

@davish davish left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @jaspwr! One comment -- new settings must be added as optional/potentially undefined in the typescript, since old users updating won't have that setting defined. Making that change will likely cause typescript errors where that undefined isn't handled, so after fixing those, I'd be happy to merge this. It would be a great addition!

src/ui/settings.ts Outdated Show resolved Hide resolved
@jaspwr
Copy link
Author

jaspwr commented Sep 7, 2023

Should be fixed now.

@davish
Copy link
Collaborator

davish commented Oct 2, 2023

I just tested this locally, and I couldn't find en-us as a locale option, so I don't think the dropdown is displaying all possible options.

@jaspwr
Copy link
Author

jaspwr commented Oct 2, 2023

I had a look at that allLocales list and it seems to be generated from all of the items in this folder (see here) and as en-us (or just en as FullCalendar calls it) is the default and the files in this folder override its properties, it's not included in this list. This might be an oversight and should probably be fixed upstream if that is the case. In the live demo they provide they use getAvailableLocaleCodes to achieve this however that method is on the actual Calendar object and as far as I know there is no Calendar instance accessible in settings.ts. Of course there's the hacky solution of just appending "en" to the list but I'll have a better look at this later and try find the best solution. Otherwise I can just try get allLocales fixed upstream.

@jaspwr
Copy link
Author

jaspwr commented Oct 17, 2023

I changed it to add en to the list manually in 7be9a8c and gave it the label en-us because I think that is less confusing.

@gfpoliva
Copy link

Hey @davish, isn't it ready to merge?

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