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

amp-date-picker: restore rrule support #28887

Merged
merged 15 commits into from
Jul 1, 2020
1 change: 1 addition & 0 deletions build-system/compile/sources.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const COMMON_GLOBS = [
'node_modules/intersection-observer/intersection-observer.install.js',
'node_modules/promise-pjs/package.json',
'node_modules/promise-pjs/promise.mjs',
'node_modules/rrule/dist/es5/rrule.min.js',
Copy link
Contributor

Choose a reason for hiding this comment

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

OMG, you got this to work!

giphy-downsized

'node_modules/web-animations-js/package.json',
'node_modules/web-animations-js/web-animations.install.js',
'node_modules/web-activities/package.json',
Expand Down
15 changes: 15 additions & 0 deletions build-system/externs/amp.extern.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,21 @@ window.vg;
*/
let ReactRender = function () {};

/** @constructor */
let RRule;
/**
* @param {Date} unusedDt
* @param {boolean=} unusedInc
* @return {?Date}
*/
RRule.prototype.before = function (unusedDt, unusedInc) {};
/**
* @param {Date} unusedDt
* @param {boolean=} unusedInc
* @return {?Date}
*/
RRule.prototype.after = function (unusedDt, unusedInc) {};

/**
* @dict
*/
Expand Down
24 changes: 24 additions & 0 deletions build-system/tasks/update-packages.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,29 @@ function patchIntersectionObserver() {
writeIfUpdated(patchedName, file);
}

/**
* TODO(samouri): remove this patch when a better fix is upstreamed (https://github.com/jakubroztocil/rrule/pull/410).
*
* Patches rrule to remove references to luxon. Even though rrule marks luxon as an optional dependency,
* it is used as if it's a required one (static import). rrule relies on its consumers either
* installing luxon or adding it as a webpack-style external. We don't want the former and
* can't yet do the latter.
*
* This function replaces the reference to luxon with a mock that throws (which the code handles well).
*/
function patchRRule() {
const path = 'node_modules/rrule/dist/es5/rrule.min.js';
const patchedContents = fs
.readFileSync(path)
.toString()
.replace(
/require\("luxon"\)/g,
`{ DateTime: { fromJSDate() { throw TypeError() } } }`
);

writeIfUpdated(path, patchedContents);
}

/**
* Does a yarn check on node_modules, and if it is outdated, runs yarn.
*/
Expand Down Expand Up @@ -145,6 +168,7 @@ async function updatePackages() {
}
patchWebAnimations();
patchIntersectionObserver();
patchRRule();
}

module.exports = {
Expand Down
13 changes: 13 additions & 0 deletions examples/date-picker.amp.html
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,19 @@
</head>
<body>
<h1>amp-date-picker</h1>
<h2>highlighted and blocked attributes</h2>
<amp-date-picker id="simple-date-picker-3"
type="single"
mode="static"
layout="fixed-height"
height="360"
on="select:AMP.setState({date3: event.date ? event.date : ''})"
locale="en"
format="YYYY-MM-DD"
highlighted="FREQ=WEEKLY;WKST=SU;BYDAY=TH"
blocked="FREQ=WEEKLY;WKST=SU;BYDAY=SA,SU">
</amp-date-picker>

<h2><code>&lt;amp-date-range&gt;</code></h2>
<p>Try to pick a range. This is a date-range picker</p>
<amp-date-picker
Expand Down
84 changes: 79 additions & 5 deletions extensions/amp-date-picker/0.1/dates-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,20 @@
* limitations under the License.
*/

import * as rrule from '../../../node_modules/rrule/dist/es5/rrule.min.js';
import {requireExternal} from '../../../src/module';

const rrulestr = rrule.default.rrulestr || rrule.rrulestr; // CC imports into .default, browserify flattens a layer.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Please move this into tryParseRrulestr.

Copy link
Member Author

@samouri samouri Jun 30, 2020

Choose a reason for hiding this comment

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

I think it makes more sense where it is, considering ideally it would even be import {rrulestr} from 'rrule';. We can discuss and i'd be happy to move it in a teeny future PR if it makes the most sense / I misunderstood the reasoning


/** @enum {string} */
const DateType = {
INVALID: 'invalid',
RRULE: 'rrule',
DATE: 'date',
};

/**
* A class which wraps a list of moment dates.
* A class which wraps a list of moment dates or RRULE dates.
*/
export class DatesList {
/**
Expand All @@ -38,6 +42,11 @@ export class DatesList {
/** @private @const */
this.moment_ = requireExternal('moment');

/** @private @const */
this.rrulestrs_ = dates
.filter((d) => this.getDateType_(d) === DateType.RRULE)
.map((d) => tryParseRrulestr(d));

/** @private @const */
this.dates_ = dates
.filter((d) => this.getDateType_(d) == DateType.DATE)
Expand All @@ -46,14 +55,14 @@ export class DatesList {
}

/**
* Determines if the given date is contained within the moment dates
* Determines if the given date is contained within the RRULEs or moment dates
* contained in the date list.
* @param {!moment|string} date
* @return {boolean}
*/
contains(date) {
const m = this.moment_(date);
return this.matchesDate_(m);
return this.matchesDate_(m) || this.matchesRrule_(m);
}

/**
Expand All @@ -73,7 +82,17 @@ export class DatesList {
}
}

return firstDatesAfter.sort((a, b) => a.toDate() - b.toDate())[0];
const rruleDates = this.rrulestrs_
.map((rrule) => /** @type {RRule} */ (rrule).after(date))
.filter(Boolean)
.map(normalizeRruleReturn);

return firstDatesAfter.concat(rruleDates).sort((a, b) => {
// toDate method does not exist for RRule dates.
a = a.toDate ? a.toDate() : a;
b = b.toDate ? b.toDate() : b;
return a - b;
})[0];
}

/**
Expand All @@ -87,7 +106,26 @@ export class DatesList {
}

/**
* Distinguish between moment dates.
* Determines if any internal RRULE object matches the given date.
* @param {!moment} date
* @return {boolean}
* @private
*/
matchesRrule_(date) {
const nextDate = date.clone().startOf('day').add(1, 'day').toDate();
return this.rrulestrs_.some((rrule) => {
const rruleUTCDate = /** @type {RRule} */ (rrule).before(nextDate);
if (!rruleUTCDate) {
return false;
}
const rruleLocalDate = normalizeRruleReturn(rruleUTCDate);
const rruleMoment = this.moment_(rruleLocalDate);
return this.ReactDates_['isSameDay'](rruleMoment, date);
});
}

/**
* Distinguish between RRULE dates and moment dates.
* @param {!moment|string} date
* @return {!DateType}
* @private
Expand All @@ -97,6 +135,42 @@ export class DatesList {
return DateType.DATE;
}

const dateStr = /** @type {string} */ (date);
if (tryParseRrulestr(dateStr)) {
return DateType.RRULE;
}

return DateType.INVALID;
}
}

/**
* RRULE returns dates as local time formatted at UTC, so the
* Date.prototype.getUTC* methods must be used to create a new date object.
* {@link https://github.com/jakubroztocil/rrule#important-use-utc-dates}
* @param {!Date} rruleDate
* @return {!Date}
*/
function normalizeRruleReturn(rruleDate) {
const year = rruleDate.getUTCFullYear();
const month = rruleDate.getUTCMonth();
const day = rruleDate.getUTCDate();
const hours = rruleDate.getUTCHours();
const minutes = rruleDate.getUTCMinutes();
const seconds = rruleDate.getUTCSeconds();
const ms = rruleDate.getUTCMilliseconds();
return new Date(year, month, day, hours, minutes, seconds, ms);
}

/**
* Tries to parse a string into an RRULE object.
* @param {string} str A string which represents a repeating date RRULE spec.
* @return {?RRule}
*/
function tryParseRrulestr(str) {
try {
return rrulestr(str, {});
} catch (e) {
return null;
}
}
8 changes: 5 additions & 3 deletions extensions/amp-date-picker/0.1/test/test-amp-date-picker.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ describes.realWin(
"templates": [{
"id": "srcTemplate",
"dates": [
"2018-01-01"
"2018-01-01",
"FREQ=WEEKLY;DTSTART=20180101T080000Z;WKST=SU;BYDAY=WE"
]
}, {
"id": "defaultTemplate"
Expand Down Expand Up @@ -232,7 +233,7 @@ describes.realWin(

describe('templates', () => {
describe('element templates', () => {
it('should parse date templates', () => {
it('should parse RRule and date templates', () => {
const template = createDateTemplate('{{template}}', {
dates: '2018-01-01',
});
Expand All @@ -245,7 +246,7 @@ describes.realWin(
});

describe('src templates', () => {
it('should parse date templates', function () {
it('should parse RRULE and date templates', function () {
this.timeout(4000);
const template = createDateTemplate('{{val}}', {
dates: '2018-01-01',
Expand All @@ -263,6 +264,7 @@ describes.realWin(
);
expect(srcTemplates[0].dates.contains('2018-01-01')).to.be.true;
expect(srcTemplates[0].dates.contains('2018-01-02')).to.be.false;
expect(srcTemplates[0].dates.contains('2018-01-03')).to.be.true;
expect(srcTemplates[0].template).to.equal(template);
expect(srcDefaultTemplate).to.equal(defaultTemplate);
});
Expand Down
16 changes: 12 additions & 4 deletions extensions/amp-date-picker/0.1/test/test-dates-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,26 @@ import {requireExternal} from '../../../../src/module';
describes.sandboxed('DatesList', {}, () => {
const moment = requireExternal('moment');

it('should accept date strings', function () {
it('should accept date strings and RRule strings', function () {
this.timeout(3000);
const containedDate = '09/04/1998';
const notContainedDate = '09/03/1998';
const datesList = new DatesList([containedDate]);
const containedRrule =
'FREQ=WEEKLY;COUNT=10;DTSTART=20180101T000000Z;WKST=SU;BYDAY=TU,SA';
const matchesRrule = '01/02/2018';
const datesList = new DatesList([containedDate, containedRrule]);

expect(datesList.contains(containedDate)).to.be.true;
expect(datesList.contains(notContainedDate)).to.be.false;
expect(datesList.contains(matchesRrule)).to.be.true;
});

it('should accept moment objects', () => {
const containedDate = '09/04/1998';
const containedMoment = moment(containedDate);
const datesList = new DatesList([containedMoment]);
const containedRrule =
'FREQ=WEEKLY;COUNT=10;DTSTART=20180101T000000Z;WKST=SU;BYDAY=TU,SA';
const datesList = new DatesList([containedMoment, containedRrule]);

expect(datesList.contains(containedDate)).to.be.true;
expect(datesList.contains(containedMoment)).to.be.true;
Expand Down Expand Up @@ -62,8 +68,10 @@ describes.sandboxed('DatesList', {}, () => {
const containedDate = '09/04/1998';
const dateBefore = '01/01/1998';
// const notContainedDate = '09/03/1998';
const containedRrule =
'FREQ=WEEKLY;COUNT=10;DTSTART=20180101T000000Z;WKST=SU;BYDAY=TU,SA';
// const matchesRrule = '01/02/2018';
const datesList = new DatesList([containedDate]);
const datesList = new DatesList([containedDate, containedRrule]);

const result = datesList.firstDateAfter(dateBefore);

Expand Down
22 changes: 15 additions & 7 deletions extensions/amp-date-picker/amp-date-picker.md
Original file line number Diff line number Diff line change
Expand Up @@ -183,12 +183,20 @@ and the user can select a date range with a starting date and ending date.

## Date formats

`amp-date-picker` attributes accept dates in ISO 8601.
`amp-date-picker` attributes accept dates in ISO 8601 and RFC 5545 RRULE formats.

[ISO 8601](https://en.wikipedia.org/wiki/ISO_8601) formats dates as `YYYY-MM-DD`
and is the standard for sharing dates between electronic systems.
For example, ISO 8601 formats the date February 28 2018 as `2018-02-28`.

[RFC 5545 Recurrence Rules (RRULEs)](https://icalendar.org/iCalendar-RFC-5545/3-3-10-recurrence-rule.html)
standardize a format for specifying repeating dates.
For example, RFC 5545 formats Halloween as `RRULE:FREQ=YEARLY;BYMONTH=10;BYMONTHDAY=31`.
More complex dates are also possible, such as the United States Thanksgiving holiday,
which is every November on the fourth Thursday: `RRULE:FREQ=YEARLY;BYMONTH=11;BYDAY=+4TH`.
The API is not friendly to memorize, but there are various
[RRULE generators](https://jakubroztocil.github.io/rrule) available online.

## Attributes

##### mode
Expand Down Expand Up @@ -349,11 +357,11 @@ The day to specify as the first day of the week (0-6). The default value is `"0"

##### blocked

A space-separated list of ISO 8601 dates to prevent the user from selecting on the calendar.
A space-separated list of ISO 8601 dates or RFC 5545 RRULE repeating dates to prevent the user from selecting on the calendar.

##### highlighted

A space-separated list of ISO 8601 dates to specially style as highlighted to draw the user's attention.
A space-separated list of ISO 8601 dates or RFC 5545 RRULE repeating dates to specially style as highlighted to draw the user's attention.
Default styling is a blue dot on the date.

##### day-size
Expand Down Expand Up @@ -398,7 +406,7 @@ The following table lists the properties that you can specify in the JSON data:
</tr>
<tr>
<td><code>blocked</code></td>
<td>An array of ISO 8601 single dates to render as blocked in the calendar view. The user is prevented from selecting these dates.</td>
<td>An array of ISO 8601 single dates or RFC 5545 RRULE repeating dates to render as blocked in the calendar view. The user is prevented from selecting these dates.</td>
</tr>
<tr>
<td><code>date</code></td>
Expand All @@ -410,7 +418,7 @@ The following table lists the properties that you can specify in the JSON data:
</tr>
<tr>
<td><code>highlighted</code></td>
<td>An array of ISO 8601 single dates to render as highlighted in the calendar view.</td>
<td>An array of ISO 8601 single dates or RFC 5545 RRULE repeating dates to render as highlighted in the calendar view.</td>
</tr>
<tr>
<td><code>startDate</code></td>
Expand All @@ -427,7 +435,7 @@ The `src` attribute may be updated after a user gesture with [`amp-bind`](https:

###### template definition objects

The `dates` property is an array of ISO 8601 single dates.
The `dates` property is an array of ISO 8601 single dates or RFC 5545 RRULE repeating dates.
The `id` property specifies the `id` of a template that the date picker can use to
render the specified dates in the calendar view.

Expand Down Expand Up @@ -736,7 +744,7 @@ Using `src` prevents chached AMP documents from showing out-of-date information.

A `date-template` must have a `dates` or `default` attribute.

- **dates**: A space-separated list of ISO 8601 single dates.
- **dates**: A space-separated list of ISO 8601 single dates or RFC 5545 RRULE repeating dates.
The template content will render for the dates matching the dates in the attribute.
- **default**: If the `default` attribute is present, the template content will render for
all dates not matching an existing template.
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"promise-pjs": "1.1.4",
"prop-types": "15.7.2",
"react-dates": "15.5.3",
"rrule": "2.6.4",
"web-activities": "1.13.0",
"web-animations-js": "2.3.1"
},
Expand Down
7 changes: 7 additions & 0 deletions test/fixtures/e2e/amp-date-picker/blocked-dates.html
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,12 @@ <h2>blocked dates</h2>
number-of-months="2"
src="http://localhost:__TEST_SERVER_PORT__/test/date-picker/config.json">
</amp-date-picker>
<amp-date-picker
id="rrule"
layout="fixed-height" height="360"
number-of-months="2"
highlighted="FREQ=WEEKLY;WKST=SU;BYDAY=TH"
blocked="FREQ=WEEKLY;WKST=SU;BYDAY=SA,SU">
</amp-date-picker>
</body>
</html>
Loading