Skip to content

Commit

Permalink
[amp-date-display] Tests and content imporvements.
Browse files Browse the repository at this point in the history
- Added invalid test cases.
- Throws user error if invalid data-options-* param is provided.
- Updated data-options-* readme doc.
- Added new lines to .out validator files.
- Applied reviewer suggestions.
  • Loading branch information
jingfei committed May 26, 2021
1 parent b7bcffb commit 384bf20
Show file tree
Hide file tree
Showing 8 changed files with 148 additions and 69 deletions.
101 changes: 52 additions & 49 deletions extensions/amp-date-display/0.1/amp-date-display.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
import {AmpEvents} from '../../../src/core/constants/amp-events';
import {Services} from '../../../src/services';
import {createCustomEvent} from '../../../src/event-helper';
import {dev, devAssert, userAssert} from '../../../src/log';
import {dashToCamelCase} from '../../../src/core/types/string/index.js';
import {dev, devAssert, user, userAssert} from '../../../src/log';
import {isLayoutSizeDefined} from '../../../src/layout';
import {removeChildren} from '../../../src/dom';

Expand All @@ -30,30 +31,6 @@ const DEFAULT_LOCALE = 'en';
/** @const {number} */
const DEFAULT_OFFSET_SECONDS = 0;

/** @const {!Array<string>} */
const SUPPORTED_LOCALE_OPTIONS_ATTRS = [
'date-style',
'time-style',
'calendar',
'day-period',
'numbering-system',
'locale-matcher',
'time-zone',
'hour12',
'hour-cycle',
'format-matcher',
'weekday',
'era',
'year',
'month',
'day',
'hour',
'minute',
'second',
'fractional-second-digits',
'time-zone-name',
];

/** @const {!Object<string, *>} */
const DEFAULT_DATETIME_OPTIONS = {
'year': 'numeric',
Expand Down Expand Up @@ -166,7 +143,10 @@ export class AmpDateDisplay extends AMP.BaseElement {

this.locale_ = this.element.getAttribute('locale') || DEFAULT_LOCALE;

this.localeOptions_ = this.parseLocalOptionsAttrs_(this.element);
this.localeOptions_ = this.parseLocaleOptionsAttrs_(
this.element,
'data-options-'
);

const data = /** @type {!JsonObject} */ (this.getDataForTemplate_());
this.templates_
Expand Down Expand Up @@ -224,34 +204,57 @@ export class AmpDateDisplay extends AMP.BaseElement {
return epoch;
}

/**
* @param {null|string} attributeName
* @param {string|undefined} attributePrefix
* @return {boolean}
* @private
*/
matchesAttrPrefix_(attributeName, attributePrefix) {
return (
attributeName !== null &&
attributePrefix !== undefined &&
attributeName.startsWith(attributePrefix) &&
attributeName !== attributePrefix
);
}

/**
* @param {!Element} element
* @param {string} attrPrefix
* @return {Object<string, *>|undefined}
* @private
*/
parseLocalOptionsAttrs_(element) {
const getCamelCase = (kebabCase) =>
kebabCase
.split('-')
.map((str, index) =>
index === 0 ? str : str[0].toUpperCase() + str.substring(1)
)
.join('');

const localeOptions = SUPPORTED_LOCALE_OPTIONS_ATTRS.filter((attr) =>
element.hasAttribute(`data-options-${attr}`)
);
if (localeOptions.length === 0) {
return undefined;
parseLocaleOptionsAttrs_(element, attrPrefix) {
const currObj = {};
let objContains = false;
const attrs = element.attributes;
for (let i = 0; i < attrs.length; i++) {
const attrib = attrs[i];
if (this.matchesAttrPrefix_(attrib.name, attrPrefix)) {
currObj[dashToCamelCase(attrib.name.slice(attrPrefix.length))] =
attrib.value;
objContains = true;
}
}
if (objContains) {
return currObj;
}
}

return localeOptions.reduce(
(options, attr) => ({
...options,
[getCamelCase(attr)]: element.getAttribute(`data-options-${attr}`),
}),
{}
);
/**
* @param {!Date} date
* @param {string} locale
* @param {?Object<string, *>} localeOptions
* @return {string}
* @private
*/
getLocaleString_(date, locale, localeOptions) {
try {
return date.toLocaleString(locale, localeOptions);
} catch (e) {
user().error(TAG, 'localeOptions', e);
}
}

/**
Expand All @@ -278,7 +281,7 @@ export class AmpDateDisplay extends AMP.BaseElement {
minute: date.getMinutes(),
second: date.getSeconds(),
iso: date.toISOString(),
localeString: date.toLocaleString(locale, localeOptions),
localeString: this.getLocaleString_(date, locale, localeOptions),
};
}

Expand Down Expand Up @@ -318,7 +321,7 @@ export class AmpDateDisplay extends AMP.BaseElement {
minute: date.getUTCMinutes(),
second: date.getUTCSeconds(),
iso: date.toISOString(),
localeString: date.toLocaleString(locale, localeOptionsInUTC),
localeString: this.getLocaleString_(date, locale, localeOptionsInUTC),
};
}

Expand Down
3 changes: 3 additions & 0 deletions extensions/amp-date-display/0.1/amp-date-display.md
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ The `data-options-*` supports all the options under [Intl.DateTimeFormat.options
parameter that specifies the formatting style to use for `localeString` format.
Valid attributes include: `data-options-date-style`, `data-options-time-style`, etc.

Note that if `display-in` attrubute is set to `utc`, the value of
`data-options-time-zone` will automatically be converted to `UTC`.

## Validation

See [amp-date-display rules](../validator-amp-date-display.protoascii) in the AMP validator specification.
49 changes: 33 additions & 16 deletions extensions/amp-date-display/0.1/test/test-amp-date-display.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
import '../amp-date-display';
import * as fakeTimers from '@sinonjs/fake-timers';
import {Services} from '../../../../src/services';
import {expect} from 'chai';
import {user} from '../../../../src/log';

describes.realWin(
'amp-date-display',
Expand Down Expand Up @@ -175,6 +177,31 @@ describes.realWin(
'2001-02-03T04:05:06.007Z'
);
});

it('locale and data-options-time-style', async () => {
element.setAttribute('datetime', '2001-02-03T04:05:06.007Z');
element.setAttribute('display-in', 'UTC');
element.setAttribute('locale', 'zh-TW');
element.setAttribute('data-options-time-style', 'short');
await element.buildInternal();

const data = impl.getDataForTemplate_();

expect(data.localeString).to.equal('上午4:05');
});

it('locale, data-options-time-style, and data-options-date-style', async () => {
element.setAttribute('datetime', '2001-02-03T04:05:06.007Z');
element.setAttribute('display-in', 'UTC');
element.setAttribute('locale', 'zh-TW');
element.setAttribute('data-options-date-style', 'full');
element.setAttribute('data-options-time-style', 'medium');
await element.buildInternal();

const data = impl.getDataForTemplate_();

expect(data.localeString).to.equal('2001年2月3日 星期六 上午4:05:06');
});
});

it('adds offset seconds', async () => {
Expand Down Expand Up @@ -209,29 +236,19 @@ describes.realWin(
expect(data.dayNameShort).to.equal('so');
});

it('locale and data-options-time-style', async () => {
it('throws error when invalid data-options value is provided', async () => {
const spy = env.sandbox.stub(user(), 'error');
element.setAttribute('datetime', '2001-02-03T04:05:06.007Z');
element.setAttribute('display-in', 'UTC');
element.setAttribute('locale', 'zh-TW');
element.setAttribute('data-options-time-style', 'short');
await element.buildInternal();
element.setAttribute('data-options-time-style', 'invalid');

const data = impl.getDataForTemplate_();

expect(data.localeString).to.equal('上午4:05');
});

it('locale, data-options-time-style, and data-options-date-style', async () => {
element.setAttribute('datetime', '2001-02-03T04:05:06.007Z');
element.setAttribute('display-in', 'UTC');
element.setAttribute('locale', 'zh-TW');
element.setAttribute('data-options-date-style', 'full');
element.setAttribute('data-options-time-style', 'medium');
await element.buildInternal();

const data = impl.getDataForTemplate_();

expect(data.localeString).to.equal('2001年2月3日 星期六 上午4:05:06');
expect(spy.args[0][1]).to.equal('localeOptions');
expect(spy.args[0][2]).to.match(/RangeError/);
expect(data.localeString).to.be.undefined;
});
}
);
Original file line number Diff line number Diff line change
Expand Up @@ -199,4 +199,4 @@ amp-date-display/0.1/test/validator-amp-date-display.html:173:2 The attribute 'd
| </amp-date-display>
| </body>
|
| </html>
| </html>
23 changes: 21 additions & 2 deletions extensions/amp-date-display/1.0/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ import {Wrapper, useRenderer} from '../../../src/preact/component';
import {getDate} from '../../../src/core/types/date';
import {useMemo} from '../../../src/preact';
import {useResourcesNotify} from '../../../src/preact/utils';
import {user} from '../../../src/log';

/** @const {string} */
const TAG = 'amp-date-display';

/** @const {string} */
const DEFAULT_DISPLAY_IN = 'local';
Expand Down Expand Up @@ -167,6 +171,21 @@ function enhanceBasicVariables(data) {
});
}

/**
* @param {!Date} date
* @param {string} locale
* @param {?Object<string, *>} localeOptions
* @return {string}
* @private
*/
function getLocaleString_(date, locale, localeOptions) {
try {
return date.toLocaleString(locale, localeOptions);
} catch (e) {
user().error(TAG, 'localeOptions', e);
}
}

/**
* @param {!Date} date
* @param {string} locale
Expand Down Expand Up @@ -194,7 +213,7 @@ function getVariablesInLocal(
'minute': date.getMinutes(),
'second': date.getSeconds(),
'iso': date.toISOString(),
'localeString': date.toLocaleString(locale, localeOptions),
'localeString': getLocaleString_(date, locale, localeOptions),
};
}

Expand Down Expand Up @@ -237,6 +256,6 @@ function getVariablesInUTC(
'minute': date.getUTCMinutes(),
'second': date.getUTCSeconds(),
'iso': date.toISOString(),
'localeString': date.toLocaleString(locale, localeOptionsInUTC),
'localeString': getLocaleString_(date, locale, localeOptionsInUTC),
};
}
17 changes: 17 additions & 0 deletions extensions/amp-date-display/1.0/test/test-amp-date-display.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

import '../../../amp-mustache/0.2/amp-mustache';
import '../amp-date-display';
import {expect} from 'chai';
import {user} from '../../../../src/log';
import {waitFor} from '../../../../testing/test-helper.js';
import {whenUpgradedToCustomElement} from '../../../../src/dom';

Expand Down Expand Up @@ -165,6 +167,21 @@ describes.realWin(
expect(data.localeString).to.equal('2001年2月3日 星期六 上午4:05:06');
});

it('throws error when provided invalid data-options', async () => {
const spy = env.sandbox.stub(user(), 'error');
element.setAttribute('datetime', '2001-02-03T04:05:06.007Z');
element.setAttribute('display-in', 'UTC');
element.setAttribute('locale', 'zh-TW');
element.setAttribute('data-options-date-style', 'invalid');
win.document.body.appendChild(element);

const wrapper = await waitRendered();

expect(spy.args[0][1]).to.equal('localeOptions');
expect(spy.args[0][2]).to.match(/RangeError/);
expect(wrapper.localeString).to.be.undefined;
});

it('renders default template into element', async () => {
element.setAttribute('datetime', '2001-02-03T04:05:06.007Z');
element.setAttribute('display-in', 'UTC');
Expand Down
20 changes: 20 additions & 0 deletions extensions/amp-date-display/1.0/test/test-component.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import * as Preact from '../../../../src/preact';
import {DateDisplay} from '../component';
import {mount} from 'enzyme';
import {user} from '../../../../src/log';

describes.sandboxed('DateDisplay 1.0 preact component', {}, (env) => {
let sandbox;
Expand Down Expand Up @@ -223,4 +224,23 @@ describes.sandboxed('DateDisplay 1.0 preact component', {}, (env) => {

expect(data.localeString).to.equal('上午4:05');
});

it('throws error when invalid localeOptions is passed', () => {
const spy = env.sandbox.stub(user(), 'error');
const props = {
render,
datetime: Date.parse('2001-02-03T04:05:06.007Z'),
displayIn: 'UTC',
locale: 'zh-TW',
localeOptions: {timeStyle: 'invalid'},
};
const jsx = <DateDisplay {...props} />;

const wrapper = mount(jsx);
const data = JSON.parse(wrapper.text());

expect(spy.args[0][1]).to.equal('localeOptions');
expect(spy.args[0][2]).to.match(/RangeError/);
expect(data.localeString).to.be.undefined;
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -199,4 +199,4 @@ amp-date-display/1.0/test/validator-amp-date-display.html:173:2 The attribute 'd
| </amp-date-display>
| </body>
|
| </html>
| </html>

0 comments on commit 384bf20

Please sign in to comment.