create SimpleDate and CalendarLocale objects #2839

mmalerba · 2017-01-27T23:30:59Z

No description provided.

jelbourn · 2017-01-31T03:26:00Z

jelbourn

Can you update the datepicker issue w/ a comment saying that work is happening on the datepicker branch?

jelbourn · 2017-01-31T03:27:19Z

src/lib/core/datetime/calendar-locale.ts

+import {Injectable} from '@angular/core';
+
+
+const SUPPORTS_INTL_API = !!(new Date().toLocaleDateString);


Safari 9 has toLocaleDateString but not the full Intl API.

Here's the check that modernizr uses:
https://github.com/Modernizr/Modernizr/blob/master/feature-detects/intl.js

jelbourn · 2017-01-31T03:30:38Z

src/lib/core/datetime/calendar-locale.ts

+  }
+
+  protected createArray<T>(length: number, valueFunction: (index: number) => T): T[] {
+    return Array.apply(null, Array(length)).map((v: undefined, i: number) => valueFunction(i));


Why not just Array(length).map(...) ?

Apparently that just creates an object like {length: 7} without adding keys for the indices, which causes map to not work.

jelbourn · 2017-01-31T03:35:16Z

src/lib/core/datetime/calendar-locale.ts

+        date.toNativeDate().toLocaleDateString(undefined, {year: 'numeric'}) : String(date.year);
+  }
+
+  protected createArray<T>(length: number, valueFunction: (index: number) => T): T[] {


This function is classically called range. Rename to that and move it outside the class as just a module function?

jelbourn · 2017-01-31T03:40:11Z

src/lib/core/datetime/calendar-locale.ts

+
+  protected createDaysArray(format: string) {
+    return this.createArray(7,
+        i => new Date(2017, 0, i + 1).toLocaleDateString(undefined, {weekday: format}));


According to MDN:

When formatting large numbers of dates, it is better to create an Intl.DateTimeFormat object and use the function provided by its format property.

Not sure if this counts as a "large number", but it wouldn't hurt to make the formatter

jelbourn · 2017-01-31T03:42:16Z

src/lib/core/datetime/calendar-locale.ts

+}
+
+
+export class DefaultCalendarLocale implements  CalendarLocale {


Add class description

jelbourn · 2017-01-31T03:42:37Z

src/lib/core/datetime/calendar-locale.ts

+
+
+@Injectable()
+export abstract class CalendarLocale {


Add class description

jelbourn · 2017-01-31T03:44:49Z

src/lib/core/datetime/calendar-locale.spec.ts

+
+  it('lists dates', () => {
+    expect(calendarLocale.dates).toEqual([
+      null, '1', '2', '3', '4', '5', '6', '7', '8', '9', '10', '11', '12', '13', '14', '15', '16',


Why does this start with null?

There's a comment in the base class, basically I thought it would be nice to not have to do calendarLocale.dates[date + 1] all the time (especially since months and days are 0-indexed), so I added null as the representation for dates[0]

jelbourn · 2017-01-31T03:47:37Z

src/lib/core/datetime/calendar-locale.spec.ts

+import {SimpleDate} from './simple-date';
+
+
+describe('DefaultCalendarLocale', () => {


These tests don't seem to well-capture using the current locale with fallback behavior. Any ideas on how to improve this?

unfortunately I don't think there's any way to set the browser's locale from JS

jelbourn · 2017-01-31T03:48:53Z

src/lib/core/datetime/simple-date.ts

@@ -0,0 +1,11 @@
+export class SimpleDate {


Add class description

googlebot · 2017-02-01T16:41:15Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

googlebot · 2017-02-01T16:46:26Z

CLAs look good, thanks!

mmalerba · 2017-02-01T17:39:18Z

+ @andrewseguin since kara is busy caretaking this week

andrewseguin · 2017-02-01T23:38:08Z

src/lib/core/datetime/calendar-locale.ts

+  }
+
+  private _createFormatFunction(
+      options: Object, fallback: (date: SimpleDate) => string): (date: SimpleDate) => string {


fix identations

andrewseguin · 2017-02-01T23:44:57Z

src/lib/core/datetime/calendar-locale.ts

+    return range(31, i => dtf.format(new Date(2017, 0, i + 1)));
+  }
+
+  private _createFormatFunction(


Can you leave a description of this function? It's especially difficult to visually parse the signature of the function

done, also simplified it since its just as easy to return null and specify the fallback elsewhere

* create SimpleDate and CalendarLocale objects * tests * addressed comments * make parseDate more robust * simplify createFormatFunction

angular-automatic-lock-bot · 2019-09-08T16:01:16Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

mmalerba assigned jelbourn and kara Jan 27, 2017

mmalerba requested review from jelbourn and kara January 27, 2017 23:30

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jan 27, 2017

mmalerba added the pr: needs review label Jan 30, 2017

jelbourn reviewed Jan 31, 2017

View reviewed changes

mmalerba force-pushed the dp-init branch from 9d27361 to 7013320 Compare February 1, 2017 16:41

googlebot added cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels Feb 1, 2017

mmalerba added 4 commits February 1, 2017 08:45

create SimpleDate and CalendarLocale objects

1e7b608

tests

464a610

addressed comments

251271c

make parseDate more robust

b7350de

mmalerba force-pushed the dp-init branch from 7013320 to b7350de Compare February 1, 2017 16:46

googlebot added cla: yes PR author has agreed to Google's Contributor License Agreement and removed cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla labels Feb 1, 2017

mmalerba requested a review from andrewseguin February 1, 2017 17:38

mmalerba assigned andrewseguin Feb 1, 2017

andrewseguin reviewed Feb 1, 2017

View reviewed changes

andrewseguin approved these changes Feb 1, 2017

View reviewed changes

andrewseguin added pr: lgtm action: merge The PR is ready for merge by the caretaker and removed pr: needs review labels Feb 1, 2017

mmalerba removed the action: merge The PR is ready for merge by the caretaker label Feb 2, 2017

simplify createFormatFunction

fee2019

mmalerba merged commit c2eb2a8 into angular:datepicker Feb 2, 2017

mmalerba added a commit that referenced this pull request Feb 9, 2017

create SimpleDate and CalendarLocale objects (#2839)

e88d0e7

* create SimpleDate and CalendarLocale objects * tests * addressed comments * make parseDate more robust * simplify createFormatFunction

mmalerba added a commit that referenced this pull request Feb 27, 2017

create SimpleDate and CalendarLocale objects (#2839)

1864285

* create SimpleDate and CalendarLocale objects * tests * addressed comments * make parseDate more robust * simplify createFormatFunction

mmalerba added a commit that referenced this pull request Mar 2, 2017

create SimpleDate and CalendarLocale objects (#2839)

5c2dbf0

* create SimpleDate and CalendarLocale objects * tests * addressed comments * make parseDate more robust * simplify createFormatFunction

mmalerba added a commit that referenced this pull request Mar 17, 2017

create SimpleDate and CalendarLocale objects (#2839)

9bcdb86

* create SimpleDate and CalendarLocale objects * tests * addressed comments * make parseDate more robust * simplify createFormatFunction

mmalerba added a commit that referenced this pull request Apr 14, 2017

create SimpleDate and CalendarLocale objects (#2839)

2d214ee

* create SimpleDate and CalendarLocale objects * tests * addressed comments * make parseDate more robust * simplify createFormatFunction

mmalerba added a commit that referenced this pull request Apr 20, 2017

create SimpleDate and CalendarLocale objects (#2839)

9cbb7c7

* create SimpleDate and CalendarLocale objects * tests * addressed comments * make parseDate more robust * simplify createFormatFunction

mmalerba added a commit that referenced this pull request Apr 29, 2017

create SimpleDate and CalendarLocale objects (#2839)

86df447

* create SimpleDate and CalendarLocale objects * tests * addressed comments * make parseDate more robust * simplify createFormatFunction

mmalerba added a commit that referenced this pull request May 5, 2017

create SimpleDate and CalendarLocale objects (#2839)

21b03ca

* create SimpleDate and CalendarLocale objects * tests * addressed comments * make parseDate more robust * simplify createFormatFunction

mmalerba added a commit that referenced this pull request May 9, 2017

create SimpleDate and CalendarLocale objects (#2839)

869d0f2

* create SimpleDate and CalendarLocale objects * tests * addressed comments * make parseDate more robust * simplify createFormatFunction

mmalerba deleted the dp-init branch April 3, 2018 15:15

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

create SimpleDate and CalendarLocale objects #2839

create SimpleDate and CalendarLocale objects #2839

mmalerba commented Jan 27, 2017

jelbourn commented Jan 31, 2017

jelbourn left a comment

jelbourn Jan 31, 2017

jelbourn Jan 31, 2017

mmalerba Jan 31, 2017

jelbourn Jan 31, 2017

jelbourn Jan 31, 2017

jelbourn Jan 31, 2017

jelbourn Jan 31, 2017

jelbourn Jan 31, 2017

mmalerba Jan 31, 2017

jelbourn Jan 31, 2017

mmalerba Jan 31, 2017

jelbourn Jan 31, 2017

googlebot commented Feb 1, 2017

googlebot commented Feb 1, 2017

mmalerba commented Feb 1, 2017 •

edited

Loading

andrewseguin Feb 1, 2017

andrewseguin Feb 1, 2017

mmalerba Feb 2, 2017

angular-automatic-lock-bot bot commented Sep 8, 2019

		import {Injectable} from '@angular/core';


		const SUPPORTS_INTL_API = !!(new Date().toLocaleDateString);

		}


		export class DefaultCalendarLocale implements CalendarLocale {

		import {SimpleDate} from './simple-date';


		describe('DefaultCalendarLocale', () => {

create SimpleDate and CalendarLocale objects #2839

create SimpleDate and CalendarLocale objects #2839

Conversation

mmalerba commented Jan 27, 2017

jelbourn commented Jan 31, 2017

jelbourn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

googlebot commented Feb 1, 2017

googlebot commented Feb 1, 2017

mmalerba commented Feb 1, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

angular-automatic-lock-bot bot commented Sep 8, 2019

mmalerba commented Feb 1, 2017 •

edited

Loading