Skip to content
This repository has been archived by the owner on Sep 18, 2023. It is now read-only.

Add timezone support for dates package #59

Merged
merged 4 commits into from
Sep 16, 2023
Merged

Add timezone support for dates package #59

merged 4 commits into from
Sep 16, 2023

Conversation

wuifdesign
Copy link
Contributor

Addresses mantinedev/mantine#3432

I added a timezone setting to the DatesProvider, allowing to shift the dates for the components using it. Dates are shifted back and forth by the parent component. It is also used in some helper functions as needed.

It is a bit hacky but the only possible solution to not introduce breaking changes.

@rtivital
Copy link
Owner

Please add documentation that describes how the feature works to DatesProvider page

@wuifdesign
Copy link
Contributor Author

wuifdesign commented Sep 12, 2023

Please add documentation that describes how the feature works to DatesProvider page

i allready added the setting to the datesprovider docs, but if you think a separate section is needed, i will add it. i just added it the same way as the other settings, and i thought this will be fine, as other settings like `locale? also don't have a separate section.

I just can't test the docs as i can't get them running on my machines (mac/windows)

@wuifdesign
Copy link
Contributor Author

@rtivital any idea why the tests are failing? that are all files that are not scope of this MR, are the tests failing in master branch?

@@ -12,5 +12,6 @@ components exported from `@mantine/dates` package. `DatesProvider` supports the
- `locale` – dayjs locale, note that you also need to import corresponding locale module from dayjs. Default value is `en`.
- `firstDayOfWeek` – number from 0 to 6, where 0 is Sunday and 6 is Saturday. Default value is 1 – Monday.
- `weekendDays` – an array of numbers from 0 to 6, where 0 is Sunday and 6 is Saturday. Default value is `[0, 6]` – Saturday and Sunday.
- `timezone` – a string providing the timezone. i.e. `UTC`
Copy link
Owner

Choose a reason for hiding this comment

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

The description is not clear, please update the description so that it contains answers to the following questions:

  • Is it applied to all components?
  • Is the user supposed to pass the date with timezone information?
  • What happens when the user passes a date that has a different timezone as a value to the component?
  • What is the default value?

I think this option requires a separate section in this documentation that will describe how it works in details.

Copy link
Contributor Author

@wuifdesign wuifdesign Sep 12, 2023

Choose a reason for hiding this comment

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

i added some docs, i hope that fits the needs

  • Is it applied to all components? -> docs added
  • Is the user supposed to pass the date with timezone information? some docs added, but the user cant provide it as you can still only provide a js Date object which doesn't have timezone support
  • What happens when the user passes a date that has a different timezone as a value to the component? user cant provide it as you can still only provide a js Date object which doesn't have timezone support
  • What is the default value? -> docs added

i hope the docs work as provided (as i can't get them running and so i have no chance of testing them)

@rtivital
Copy link
Owner

I'll check the issue with tests tomorrow

@wuifdesign
Copy link
Contributor Author

@rtivital do you still need something for this MR?

@rtivital
Copy link
Owner

No, I think it's fine

@rtivital rtivital merged commit 18ead2c into rtivital:master Sep 16, 2023
@rtivital
Copy link
Owner

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants