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

feat(datepicker): add possibility for a format function to Datepicker #153

Merged
merged 3 commits into from
Aug 3, 2021

Conversation

Calamari
Copy link

Proposed changes

This is described in #151. In short: This adds the ability to add a formatting function as format option to datepicker instance, which allows things like this:

M.Datepicker.init(elems, {
  format: date => new Intl.DateTimeFormat(locale).format(date)
});

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to change).

Checklist:

  • I have read the CONTRIBUTING document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link
Member

@roiLeo roiLeo left a comment

Choose a reason for hiding this comment

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

lgtm
#151 related

@Smankusors Smankusors linked an issue Jun 29, 2021 that may be closed by this pull request
Copy link
Member

@Smankusors Smankusors left a comment

Choose a reason for hiding this comment

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

LGTM. Well, the check failing because there's a problem with your commit messages

What do you think about these commit messages? @ChildishGiant

@ChildishGiant
Copy link
Member

It's just an issue of the case, which doesn't overly matter. I might change the action to only give a warning as it's not a massive deal if the commits aren't the right style.

@ray007
Copy link

ray007 commented Jun 30, 2021

Should a format-function not have a format parameter?

@Smankusors
Copy link
Member

Should a format-function not have a format parameter?

hmm wait, I don't get it. What do you mean?

@ray007
Copy link

ray007 commented Jul 1, 2021

Should a format-function not have a format parameter?

hmm wait, I don't get it. What do you mean?

Haven't yet looked at your pull request, but I guess it's probably already possible:

M.Datepicker.init(elems, {
  format: (date, fmt) => new Intl.DateTimeFormat(locale, fmt).format(date)
});

@Calamari
Copy link
Author

Calamari commented Jul 1, 2021

@ray007 and what value should the fmt parameter contain?

If you add a format function, why should it not be possible for that function to know how to format itself?

@Calamari Calamari force-pushed the datepicker_format branch 2 times, most recently from bfa957a to 5ad735c Compare July 1, 2021 13:08
@Calamari Calamari changed the title Add possibility for a format function to Datepicker feat(datepicker): add possibility for a format function to Datepicker Jul 20, 2021
@Calamari Calamari force-pushed the datepicker_format branch from 5ad735c to 27620cc Compare July 20, 2021 09:00
@Calamari
Copy link
Author

So, I finally fixed those commit linting problems :-)

Copy link

@DanielRuf DanielRuf left a comment

Choose a reason for hiding this comment

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

Just a small typo, rest looks good to me.

pug/page-contents/pickers_content.html Outdated Show resolved Hide resolved
Calamari added 3 commits July 23, 2021 13:42
To leverace browser own internationalization methods, it is useful
to add a format method instead of simple format string. That makes
things like this possible:

```js
M.Datepicker.init(elems, {
  format: date => new Intl.DateTimeFormat(locale).format(date)
});
```

Which would format the input according to the i18n rules defined
by the browser using the locale of the user.
@Calamari Calamari force-pushed the datepicker_format branch from 27620cc to 88c0b7e Compare July 23, 2021 11:43
@Calamari
Copy link
Author

So, let's see if I can finally pass the bouncer of a commit linter.

@Calamari
Copy link
Author

Calamari commented Aug 3, 2021

Anyone like to approve the workflow? :-)

@DanielRuf
Copy link

Anyone like to approve the workflow? :-)

As far as I can see it was already approved and successfully finished (2 successful checks).

grafik

@Smankusors
Copy link
Member

As far as I can see it was already approved and successfully finished (2 successful checks).

Oh yeah, I did approve it.

Hmm.... I assume the issue that mentioned by @ray007 is already solved (there is no reply since a month). Merging it now.

@Smankusors Smankusors merged commit 5175b53 into materializecss:v1-dev Aug 3, 2021
@Smankusors
Copy link
Member

Anyway, I sent an invitation to you to join the @materializecss/members team. Welcome 🎉

@Calamari
Copy link
Author

Calamari commented Aug 3, 2021

To me it said, it waited for approval for the workflows, that's why I wrote again. :-)
Thanks :-)

@DanielRuf
Copy link

To me it said, it waited for approval for the workflows, that's why I wrote again. :-)

I have probably manually triggered them a few days or weeks ago.

@Calamari Calamari deleted the datepicker_format branch August 6, 2021 09:09
@Smankusors Smankusors added the enhancement New feature or request label Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Date/Time picker with format function
6 participants