Add placeholder and pattern to daterange picker widget #32738

esoergel · 2023-03-14T14:56:50Z

Product Description

Adds a placeholder text with the expected format to the date range widget:

It also adds loose validation so things like "Jan 1, 2022" will show up red when the user starts typing them

Technical Summary

https://dimagi-dev.atlassian.net/browse/QA-4867

Feature Flag

Case Search: Enable synchronous mobile searching and case claiming

Safety Assurance

Safety story

Automated test coverage

n/a

QA Plan

Grew out of QA on #32628
https://dimagi-dev.atlassian.net/browse/QA-4867

Rollback instructions

This PR can be reverted after deploy with no further considerations

Labels & Review

Risk label is set correctly
The set of people pinged as reviewers is appropriate for the level of risk of the change

Jtang-1 · 2023-03-14T23:37:49Z

corehq/apps/cloudcare/static/cloudcare/js/formplayer/menus/views/query.js

@@ -370,6 +370,9 @@ hqDefine("cloudcare/js/formplayer/menus/views/query", function () {
                autoUpdateInput: false,
                "autoApply": true,
            });
+            this.ui.dateRange.attr("placeholder", dateFormat + separator + dateFormat);


I think we should redefine dateFormats to prioritize dateFormat in the list of allowable formats if dateFormat is the recommended input pattern.

This is because we parse the input of dateRange using dateFormats as order of priority input formats. parts = _.map(oldValue.split(separator), function (v) { return moment(v, dateFormats);

Fair point, just updated it. I'd also forgotten about that part that parses input formats for the date range picker. This means we can consistently apply the input parsing to both widgets! Working on that now

Good catch, that's a win for consistent date picker behavior!

I thought you were running into issues previously with using strict mode on date range picker based on this comment. But I now realize I think you were looking at something else #32628 (comment)

I was, because I was trying to do it in the library (as the single date picker does with parseInputDate). Turns out we already interpret the dates in the date range picker in this code though, so i can just jump in there. I did a bunch more fiddling to unify input processing. I still need to do more testing, but let me know what you think.

Dates aren't moment objects, so the first part of the conditional would be hit anyways. Besides, I don't this happens, since convertTwoDigitYear would fail hard on a Date input if one were passed in

Jtang-1

defining input parsing in one spot makes a lot of sense and great change

Jtang-1 · 2023-03-16T17:17:41Z

corehq/apps/cloudcare/static/cloudcare/js/formplayer/menus/views/query.js

                    }
+                    newValue = start.format(dateFormat) + separator + end.format(dateFormat);


nice clean up

Jtang-1 · 2023-03-16T17:53:12Z

corehq/apps/cloudcare/static/cloudcare/js/utils.js

+    var dateFormats = ['MM/DD/YYYY', 'YYYY-MM-DD', 'M/D/YYYY', 'M/D/YY', 'M-D-YYYY', 'M-D-YY', moment.defaultFormat];
+
+    /** Coerce an input date string to a moment object */
+    var parseInputDate = function (dateString) {


I feel like something weird happens if the dateString input is YYYY-MM-DD since convertTwoDigitYear will try to convert the DD into four digits. But the unit test seems to pass and i can't figure out why

Ah, no issue here. YYYY-MM-DD won't be converted because convertTwoDigitYear checks if dateString has <= 6 digits before converting

Nice, yeah that makes sense - it might even be reasonable to switch to a more strict pattern like regex matching ^\d{1,2}[.-/]\d{1,2}[.-/]\d{1,2}$. Hell, I wonder if temporarily patching moment.parseTwoDigitYear might be an option if we did something like:

oldParseTwoDigitYear = moment.parseTwoDigitYear; moment.parseTwoDigitYear = function () {...}; userDate = moment(dateString, ...); moment.parseTwoDigitYear = oldParseTwoDigitYear;

Anyways, I think iterating via unit tests is probably our best bet here, especially given that part of this is controlled somewhat opaquely by a library. Once I switched to that approach, I felt a lot more confident in this code. Honestly I should've done it much sooner, but I guess I'm just not used to that in javascript.

I'm not a huge fan of how convertTwoDigitYear requires the argument to follow some pattern/format. I'm thinking if the argument should be be just the YearString but that'll make the caller complicated. Patching seems interesting but not sure if that's a hacky solution. So not sure what a better would be.
Agreed about the tests, great addition

totally agree

Jtang-1

Pulled the change in locally and tested some cases. Didn't see any issues!

orangejenny · 2023-03-16T20:53:25Z

Is this going through QA?

esoergel · 2023-03-17T15:50:34Z

@orangejenny yep, here: https://dimagi-dev.atlassian.net/browse/QA-4817

esoergel added 2 commits March 13, 2023 17:24

Add expected daterange format as placeholder

7679e9a

Add html pattern to date range input

8166562

esoergel added the product/feature-flag Change will only affect users who have a specific feature flag enabled label Mar 14, 2023

esoergel requested a review from Jtang-1 March 14, 2023 14:56

esoergel requested a review from orangejenny as a code owner March 14, 2023 14:56

dimagimon added the Risk: Medium Change affects files that have been flagged as medium risk. label Mar 14, 2023

Jtang-1 reviewed Mar 14, 2023

View reviewed changes

Preference stated format in coercion; be strict

27a75ba

Jtang-1 approved these changes Mar 15, 2023

View reviewed changes

esoergel added 4 commits March 15, 2023 14:53

Extract parseInputDate for reuse and testing

cac6b57

Use parsed date even in special case

e1651fc

remove nonsensical conditional

7d6fa82

Dates aren't moment objects, so the first part of the conditional would be hit anyways. Besides, I don't this happens, since convertTwoDigitYear would fail hard on a Date input if one were passed in

Allow formats that don't zero-pad months and days

04cb40b

Jtang-1 self-requested a review March 15, 2023 20:25

stickler

1f0f5d2

Jtang-1 reviewed Mar 16, 2023

View reviewed changes

Jtang-1 approved these changes Mar 16, 2023

View reviewed changes

esoergel merged commit 43c0bf1 into master Mar 21, 2023

esoergel deleted the es/daterange branch March 21, 2023 16:46

esoergel added the QA Passed label Mar 21, 2023

esoergel mentioned this pull request Jul 11, 2024

[B5] Web Apps #34682

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add placeholder and pattern to daterange picker widget #32738

Add placeholder and pattern to daterange picker widget #32738

esoergel commented Mar 14, 2023

Jtang-1 Mar 14, 2023

esoergel Mar 15, 2023

Jtang-1 Mar 15, 2023 •

edited

Loading

esoergel Mar 15, 2023

Jtang-1 left a comment

Jtang-1 Mar 16, 2023

Jtang-1 Mar 16, 2023

Jtang-1 Mar 16, 2023

esoergel Mar 16, 2023 •

edited

Loading

Jtang-1 Mar 16, 2023 •

edited

Loading

esoergel Mar 16, 2023

Jtang-1 left a comment

orangejenny commented Mar 16, 2023

esoergel commented Mar 17, 2023

		}
		newValue = start.format(dateFormat) + separator + end.format(dateFormat);

Add placeholder and pattern to daterange picker widget #32738

Add placeholder and pattern to daterange picker widget #32738

Conversation

esoergel commented Mar 14, 2023

Product Description

Technical Summary

Feature Flag

Safety Assurance

Safety story

Automated test coverage

QA Plan

Rollback instructions

Labels & Review

Jtang-1 Mar 14, 2023

Choose a reason for hiding this comment

esoergel Mar 15, 2023

Choose a reason for hiding this comment

Jtang-1 Mar 15, 2023 • edited Loading

Choose a reason for hiding this comment

esoergel Mar 15, 2023

Choose a reason for hiding this comment

Jtang-1 left a comment

Choose a reason for hiding this comment

Jtang-1 Mar 16, 2023

Choose a reason for hiding this comment

Jtang-1 Mar 16, 2023

Choose a reason for hiding this comment

Jtang-1 Mar 16, 2023

Choose a reason for hiding this comment

esoergel Mar 16, 2023 • edited Loading

Choose a reason for hiding this comment

Jtang-1 Mar 16, 2023 • edited Loading

Choose a reason for hiding this comment

esoergel Mar 16, 2023

Choose a reason for hiding this comment

Jtang-1 left a comment

Choose a reason for hiding this comment

orangejenny commented Mar 16, 2023

esoergel commented Mar 17, 2023

Jtang-1 Mar 15, 2023 •

edited

Loading

esoergel Mar 16, 2023 •

edited

Loading

Jtang-1 Mar 16, 2023 •

edited

Loading