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

Implementing localization #77

Closed
wants to merge 15 commits into from
Closed

Implementing localization #77

wants to merge 15 commits into from

Conversation

Frondor
Copy link
Contributor

@Frondor Frondor commented Apr 28, 2018

First try, WIP, waiting for reviews

Besides of implementing localization, this PR adds a new way to work with the library with more flexibility without breaking changes.

A full example of the new APIs and method features

// optionally, you can use a named import to get the Dayjs class and pass your own configuration object.
import { Dayjs }, dayjs from 'dayjs'
// Instantiate your own Dayjs object, and use it anywhere in your app with the provided default config
// myDayjs.js
import { Dayjs }, dayjs from 'dayjs'
import ESP from 'dayjs/locales/es'

// You can still do
dayjs().format() // "2018-04-01T23:10:59Z"

// all of these settings are optional
const config = {
    date: new Date(), // optional
    locale: ESP, // default locale for this and cloned instances
    format: 'YYYY / MM / DD HH:ss', // default format, used for empty calls to format() and object serialization
    // more config...
}

export default new Dayjs(config)

/*
* ----------------------------------
*/
// Somewhere in your app
import myDayjs from '../myDayjs'

myDayjs.set('year', 1991).format() // "1991 / 12 / 1 09:30"
myDayjs.format('YYYY') // "2018"
myDayjs
    .set(new Date(2018, 5, 1)) // you can now pass a Date or Dayjs instance to set!
    .setLocale() // change the instance locale from spanish back to english
    .format('dddd - MMMM', {
        // override default format and WeekDays locale (only WD)
        WD: ['Mondy', 'Tuesdy', 'Wendy', 'Thursdy', 'Fridy', 'Saty', 'Sundy']
    }) // "Tuesdy - May"

// And now, objects with Dayjs instances are being serialized to a JSON string using the default format
JSON.stringify({ name: 'Jon', birthday: myDayjs }) //  '{name: "Jon", birthday: "2018 / 12 / 1 09:30"}'

closes #36

@Frondor Frondor mentioned this pull request Apr 28, 2018
@codecov-io
Copy link

codecov-io commented Apr 28, 2018

Codecov Report

Merging #77 into dev will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@         Coverage Diff         @@
##            dev    #77   +/-   ##
===================================
  Coverage   100%   100%           
===================================
  Files         3      3           
  Lines       175    188   +13     
  Branches     27     34    +7     
===================================
+ Hits        175    188   +13
Impacted Files Coverage Δ
src/index.js 100% <100%> (ø) ⬆️
src/constant.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ab2fb2...5f57485. Read the comment docs.

format(formatStr = 'YYYY-MM-DDTHH:mm:ssZ') {
const weeks = 'Sunday.Monday.Tuesday.Wednesday.Thursday.Friday.Saturday'.split('.')
const months = 'January.February.March.April.May.June.July.August.September.October.November.December'.split('.')
format(formatStr = this.$format, L = {}) {

Choose a reason for hiding this comment

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

I think this could be improved by using:

format(formatStr = this.$format, L = this.$L)

Then I think you can get rid of next lines short circuits ||. What do you think?

Copy link
Contributor Author

@Frondor Frondor Apr 30, 2018

Choose a reason for hiding this comment

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

Really depends, because you may do format.('...', { WD: [0,1,2,3,4,5,6] }) to override the WeekDays locale, but leave MONTHS untouched with the instance's default locale. You're providing overrides for certain locales, not the whole localized object.

@Frondor
Copy link
Contributor Author

Frondor commented May 2, 2018

Switching target branch to @next

@Frondor Frondor closed this May 2, 2018
@Frondor Frondor mentioned this pull request May 3, 2018
@@ -163,9 +172,15 @@ class Dayjs {
return this
}

set(string, int) {
set(unitOrDate, int) {
if (typeof unitOrDate === 'object' &&
Copy link
Owner

Choose a reason for hiding this comment

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

@Frondor

myDayjs.set(new Date(2018, 5, 1)) // you can now pass a Date or Dayjs instance to set!

Could you explain the meaning of you code here, please? Don't really understand.

Copy link
Contributor Author

@Frondor Frondor May 8, 2018

Choose a reason for hiding this comment

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

Sure, this code is kind of explained in the test case https://github.com/Frondor/dayjs/blob/5f574850d1cb75c6bd2a8b1684a94346f1d501b7/test/get-set.test.js#L69
In the past, you were limited to set certain time units over an already defined date ($d). Now that code checks whether the unitOrDate passed is either a time unit, a Date object, or a Dayjs instance. So you can set the whole date if needed, not just an unit. Useful for inheriting configurations like locale, plugins and default format, and create date objects out of a single source, and keeping the Dayjs immutable since that's how set works.
By the way, just for the record, this is the old PR, the new one is #90

Copy link
Contributor Author

@Frondor Frondor May 8, 2018

Choose a reason for hiding this comment

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

set(unitOrDate, int) {
    // if unitOrDate is a Date or Dayjs object
    if (typeof unitOrDate === 'object' &&
      unitOrDate.constructor.name.indexOf(['Date', 'Dayjs'] > -1)) {
      const self = this.clone() // don't mutate this instance
     // set $d date object depending on whether its a native Date object, or a Dayjs instance
      self.$d = unitOrDate.toDate ? unitOrDate.toDate() : unitOrDate
      return self // done
    }
    // if this is not a date object nor an int just fail silently (this code was already here)
    if (!Utils.isNumber(int)) return this
   // unitOrDate is definitely a integer (unit)
    return this.clone().mSet(unitOrDate, int)
  }

Copy link
Owner

Choose a reason for hiding this comment

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

@Frondor Got it, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Locales?
4 participants