Skip to content

Conversation

Cerothen
Copy link

@Cerothen Cerothen commented Feb 12, 2021

Proposed changes

The datepicker and timepicker have containers that are specified in different ways (datepicker = element, timepicker = document query selector string). These changes allow both to have the container specified by either an element or a query string for both elements to maintain backward compatability while also harmonizing the entry method.

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.

Allow the datepicker to also use the document selector like the timepicker
Allow Timepicker to be used with an element instead of a selector string
@Cerothen Cerothen marked this pull request as ready for review February 12, 2021 03:48
@Smankusors Smankusors added enhancement New feature or request javascript labels Feb 12, 2021
@DanielRuf DanielRuf requested a review from a team February 13, 2021 19:57
Copy link

@nicknickel nicknickel left a comment

Choose a reason for hiding this comment

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

LGTM

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.

I think the doc (jade/page-contents/pickers_content.html) for both the date picker and time picker need to be updated.

For example, in the date picker, the row with "container" in the name column, can be updated like this

Name Type Default Description
container Element || String null Specify a DOM element to render the calendar in, or a selector string for a DOM element to render the calendar in. By default it will be placed before the input.

And in the time picker doc, just replace "calendar" with "time picker" in the description.

@DanielRuf DanielRuf requested a review from a team April 21, 2021 21:38
@DanielRuf
Copy link

I think the doc (jade/page-contents/pickers_content.html) for both the date picker and time picker need to be updated.

For example, in the date picker, the row with "container" in the name column, can be updated like this

And in the time picker doc, just replace "calendar" with "time picker" in the description.

@Cerothen can you update the files according to the review by @Smankusors?

@DanielRuf DanielRuf removed the request for review from a team April 21, 2021 21:39
Cerothen added 2 commits May 8, 2021 21:18
Update documentation to reflect the way picker containers
@Cerothen
Copy link
Author

Cerothen commented May 9, 2021

I have updated the documentation as requested.

@Cerothen Cerothen requested a review from Smankusors May 9, 2021 01:22
@Smankusors Smankusors merged commit 17a8e87 into materializecss:v1-dev May 10, 2021
@Smankusors
Copy link
Member

@Cerothen Thanks for the PR

I also invited you to the materializecss org. Welcome! 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request javascript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants