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.

Sign up for GitHub

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

Jump to bottom

Add min and max date for Modus Date Input #1738

Merged
merged 18 commits into from
Oct 10, 2023
Merged

Add min and max date for Modus Date Input #1738

merged 18 commits into from
Oct 10, 2023

Conversation

yevhenkravets
Copy link
Contributor

Description

  • Update the modus web component with a parameter to support a minimum and/or maximum value
  • Validate input validation is working (max date value, min date value)
  • Min error message = “Select a date after [min value - 1 day]”
  • Max error message = “Select a date before [max value + 1 day]”
  • Disabled days in the calendar widget must show a tooltip when on hover/focus
  • Tooltip Message = min/max error message

References #1075

Type of change

  • 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 not work as expected)
  • Documentation update

How Has This Been Tested?

Provide min and/or max attr value for the input in the storybook and check manual typing or pasting. Also, check with the date picker.
Added related integration tests for the input and picker components

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

yevhenkravets and others added 7 commits September 18, 2023 11:46
@yevhenkravets
Merge pull request #1 from trimble-oss/main
0f5996e 
update main branch
@yevhenkravets
Date Picker Min Max attributes
99b9ba7
@yevhenkravets
Merge pull request #2 from trimble-oss/main
79029c3 
update main
@yevhenkravets
Merge remote-tracking branch 'origin/main' into issue-1075/date-picke…
7fea6c3 
…r-min-max
@yevhenkravets
Date Picker Min Max feature
f740600
@yevhenkravets
Merge branch 'trimble-oss:main' into main
1d36684
@yevhenkravets
Merge remote-tracking branch 'origin/main' into issue-1075/date-picke…
320505e 
…r-min-max

# Conflicts:
#	angular-workspace/projects/trimble-oss/modus-angular-components/src/lib/stencil-generated/components.ts
#	stencil-workspace/src/components/modus-date-input/modus-date-input.e2e.ts
@yevhenkravets yevhenkravets requested a review from a team as a code owner September 28, 2023 13:39
@netlify
Copy link

netlify bot commented Sep 28, 2023 •
edited
Loading

✅ Deploy Preview for modus-webcomponents ready!

Name Link
🔨 Latest commit 4049ae2
🔍 Latest deploy log https://app.netlify.com/sites/modus-webcomponents/deploys/652509efd04cbf0009c21870
😎 Deploy Preview https://deploy-preview-1738--modus-webcomponents.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse		 1 paths audited
Performance: 27 (🔴 down 1 from production)
Accessibility: 98 (no change from production)
Best Practices: 92 (no change from production)
SEO: 92 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

yevhenkravets
yevhenkravets commented Sep 28, 2023
View reviewed changes
stencil-workspace/src/components/modus-date-input/modus-date-input.tsx
@Method()
async validate(): Promise<void> {
this.validateInput(this._dateDisplay);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

public method to validate the input. For example, if the date range is fine and you clear the error message, but one of the fields is out of the min-max range you call it to check the validation on the input itself.

yevhenkravets
yevhenkravets commented Sep 28, 2023
View reviewed changes
stencil-workspace/src/components/modus-date-input/modus-date-input.tsx Outdated
min: this._formatter.parseIsoToDate(this.min),
max: this._formatter.parseIsoToDate(this.max),
minMessage: messages.min,
maxMessage: messages.max,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The calendar doesn't know anything about the format, but the tooltip message of the disabled date should be the same as the input one. Send them to the calendar with the min and max values.

yevhenkravets
yevhenkravets commented Sep 28, 2023
View reviewed changes
stencil-workspace/storybook/stories/components/modus-date-picker/modus-date-picker.stories.tsx
allowedCharsRegex: '.',
helperText: 'mmm dd, yyyy',
label: 'Single Date',
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added one more template with a picker and default min and max values.

@msankaran0712 msankaran0712 mentioned this pull request Sep 29, 2023
Closed
msankaran0712
msankaran0712 reviewed Oct 4, 2023
View reviewed changes
stencil-workspace/src/components/modus-date-input/modus-date-input.tsx Outdated
/** Handlers */
handleCalendarClick(): void {
this.calendarIconClicked.emit({
value: this.value,
type: this.type,
inputString: this._dateDisplay,
min: this._formatter.parseIsoToDate(this.min),
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest not parsing to Date objects because of timezone differences when emitting events, and I think we don't have to pass min and max because the Date picker has a state object that contains a reference to the date input node, we can directly consume date input properties in that. Passing the type only matters here.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would have been better to pass a reference to the element instead of its type and value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I'll try that

Copy link
Contributor

@msankaran0712 msankaran0712 Oct 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@yevhenkravets Are we working on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yevhenkravets Are we working on this?

Yes, I'll provide changes soon

@msankaran0712
Copy link
Contributor

@yevhenkravets, I have multiple PRs. If @cjwinsor can review and approve, that would be fine too👍🏼

@yevhenkravets yevhenkravets changed the title Issue 1075/date picker min max EPD-11512 Issue 1075/date picker min max Oct 5, 2023
@yevhenkravets
Merge branch 'main' into issue-1075/date-picker-min-max
d7e6254 
# Conflicts:
#	stencil-workspace/src/components/modus-date-picker/utils/modus-date-picker.state.tsx
@yevhenkravets
removed unused import
b181711
cjwinsor
cjwinsor previously approved these changes Oct 9, 2023
View reviewed changes
msankaran0712
msankaran0712 previously approved these changes Oct 10, 2023
View reviewed changes
yevhenkravets and others added 2 commits October 10, 2023 11:20
@yevhenkravets
Merge branch 'trimble-oss:main' into main
be34f74
@yevhenkravets
Merge branch 'main' into issue-1075/date-picker-min-max
4049ae2 
# Conflicts:
#	stencil-workspace/src/components/modus-date-input/modus-date-input.e2e.ts
@msankaran0712 msankaran0712 merged commit c8ff88b into trimble-oss:main Oct 10, 2023
10 checks passed
@msankaran0712 msankaran0712 changed the title EPD-11512 Issue 1075/date picker min max Add min and max date for Modus Date Input Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msankaran0712 msankaran0712 msankaran0712 approved these changes

@cjwinsor cjwinsor cjwinsor left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@yevhenkravets @msankaran0712 @cjwinsor