Skip to content
This repository was archived by the owner on Dec 27, 2022. It is now read-only.

Fix date handling compatibility with Safari#134

Merged
westonruter merged 1 commit into
developfrom
fix/safari-date-handling
Apr 18, 2017
Merged

Fix date handling compatibility with Safari#134
westonruter merged 1 commit into
developfrom
fix/safari-date-handling

Conversation

@westonruter
Copy link
Copy Markdown
Contributor

I opened the customizer in Safari and I found:

[Error] TypeError: null is not an object (evaluating 'parsed[fieldName]') at (anonymous function) (customize-snapshots.js:774)

It looks like some logic from Customize Posts was adopted before fixes for its Safari issues were implemented. See:
xwp/wp-customize-posts#293
xwp/wp-customize-posts#304

@westonruter westonruter added this to the 0.6.0 milestone Apr 17, 2017
@westonruter westonruter requested a review from mohdsayed April 17, 2017 22:52
Comment thread js/customize-snapshots.js
formattedDate += '-' + ( '00' + String( date.getDate() ) ).substr( -nonYearLength, nonYearLength );
formattedDate += ' ' + ( '00' + String( date.getHours() ) ).substr( -nonYearLength, nonYearLength );
formattedDate += ':' + ( '00' + String( date.getMinutes() ) ).substr( -nonYearLength, nonYearLength );
formattedDate += ':' + ( '00' + String( date.getSeconds() ) ).substr( -nonYearLength, nonYearLength );
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Being explicit about types just in case.

Comment thread js/customize-snapshots.js
return parseInt( datePart, 10 );
} );
return new Date( dateParts[0], dateParts[1] - 1, dateParts[2], dateParts[3], dateParts[4], dateParts[5] ); // eslint-disable-line no-magic-numbers
},
Copy link
Copy Markdown
Contributor

@mohdsayed mohdsayed Apr 18, 2017

Choose a reason for hiding this comment

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

I wish we could pass an array in the Date object, but one day we will be able to do it using spread syntax, like this, ( though it works in chrome ) 😀

return new Date(...dateParts);

Comment thread js/customize-snapshots.js
timestampDifferential;

var snapshot = this, currentDate, currentTimestamp, timestampDifferential;
currentTimestamp = ( new Date() ).valueOf();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just for my knowledge, thinking why you chose ( new Date() ).valueOf() over ( new Date() ).getTime();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good question. I don't have a particular reason. I just usually use it, as has the same output:

You can use this method to help assign a date and time to another Date object. This method is functionally equivalent to the valueOf() method.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/getTime

I agree that getTime() would perhaps look more semantic.

@westonruter westonruter merged commit 28e0084 into develop Apr 18, 2017
@westonruter westonruter deleted the fix/safari-date-handling branch April 18, 2017 15:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants