Skip to content

Conversation

@andrewn
Copy link
Member

@andrewn andrewn commented Sep 6, 2020

  • centralise all date formatting into two utility functions (dates.format(), dates.distanceInWordsToNow())
  • upgrade app to use date-fns v2 which supports localisation
  • use date-fns' locales to translate into English/Spanish

Both locales are imported into the bundle at the moment. They're only 1.5k each so not a huge issue but as we add more languages we might want to dynamically import each locale as needed. This will take more work as the components expect the formatting to be synchronous at the moment.

Fixes #1584

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • is from a uniquely-named feature branch and has been rebased on top of the latest master. (If I was asked to make more changes, I have made sure to rebase onto master then too)
  • is descriptively named and links to an issue number, i.e. Fixes #1584

@andrewn andrewn requested review from catarak and oruburos September 7, 2020 09:39
@oruburos
Copy link
Collaborator

oruburos commented Sep 7, 2020

Hi @andrewn
I checked the code locally.

Trying to test the saved ago functionality I found the following messages displayed in console:

Starting with v2.0.0-beta.1 date-fns doesn't accept strings as date arguments. Please use parseISO to parse strings. See: https://git.io/fjule
Error:
    at toDate (D:\gsocP5\p5.js-web-editor-master\src\p5.js-web-editor\node_modules\date-fns\toDate\index.js:56:20)
    at isAfter (D:\gsocP5\p5.js-web-editor-master\src\p5.js-web-editor\node_modules\date-fns\isAfter\index.js:39:42)
    at D:\gsocP5\p5.js-web-editor-master\src\p5.js-web-editor\server\controllers/project.controller.js:23:31
    at D:\gsocP5\p5.js-web-editor-master\src\p5.js-web-editor\node_modules\mongoose\lib\model.js:4828:16
    at D:\gsocP5\p5.js-web-editor-master\src\p5.js-web-editor\node_modules\mongoose\lib\query.js:4390:12
    at D:\gsocP5\p5.js-web-editor-master\src\p5.js-web-editor\node_modules\mongoose\lib\query.js:2869:28
    at processTicksAndRejections (internal/process/task_queues.js:79:11)
(node:5216) DeprecationWarning: Mongoose: findOneAndUpdate() and findOneAndDelete() without the useFindAndModify option set to false are deprecated. See: https://mongoosejs.com/docs/deprecations.html#-findandmodify-
Starting with v2.0.0-beta.1 date-fns doesn't accept strings as date arguments. Please use parseISO to parse strings. See: https://git.io/fjule
Error:
    at toDate (D:\gsocP5\p5.js-web-editor-master\src\p5.js-web-editor\node_modules\date-fns\toDate\index.js:56:20)
    at isAfter (D:\gsocP5\p5.js-web-editor-master\src\p5.js-web-editor\node_modules\date-fns\isAfter\index.js:39:42)
    at D:\gsocP5\p5.js-web-editor-master\src\p5.js-web-editor\server\controllers/project.controller.js:23:31
    at D:\gsocP5\p5.js-web-editor-master\src\p5.js-web-editor\node_modules\mongoose\lib\model.js:4828:16
    at D:\gsocP5\p5.js-web-editor-master\src\p5.js-web-editor\node_modules\mongoose\lib\query.js:4390:12
    at D:\gsocP5\p5.js-web-editor-master\src\p5.js-web-editor\node_modules\mongoose\lib\query.js:2869:28
    at processTicksAndRejections (internal/process/task_queues.js:79:11) 

@andrewn
Copy link
Member Author

andrewn commented Sep 7, 2020

Starting with v2.0.0-beta.1 date-fns doesn't accept strings as date arguments. Please use parseISO to parse strings. See: https://git.io/fjule
Error:
    at toDate (D:\gsocP5\p5.js-web-editor-master\src\p5.js-web-editor\node_modules\date-fns\toDate\index.js:56:20)
...

Good spot! I've pushed a new commit that removes this warning.

@oruburos
Copy link
Collaborator

oruburos commented Sep 7, 2020

Great, indeed, the translations for dates work.

Copy link
Collaborator

@oruburos oruburos left a comment

Choose a reason for hiding this comment

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

Great, indeed, the translations for dates work. Timer messages in the bar are not displayed but I checked against messages in console and they were correct. So functionality for this PR is there.

@catarak
Copy link
Member

catarak commented Sep 10, 2020

Great, indeed, the translations for dates work. Timer messages in the bar are not displayed but I checked against messages in console and they were correct. So functionality for this PR is there.

I tested this and I'm also not seeing the timer messages!

@andrewn
Copy link
Member Author

andrewn commented Sep 21, 2020

Great, indeed, the translations for dates work. Timer messages in the bar are not displayed but I checked against messages in console and they were correct. So functionality for this PR is there.

I tested this and I'm also not seeing the timer messages!

I couldn't see the Timer, but I think that's unrelated to this PR as I don't see it in the develop branch either. Or on the deployed editor.

Do you know what the behaviour of the Timer is and I'll try and fix it....!

@catarak
Copy link
Member

catarak commented Sep 21, 2020

I couldn't see the Timer, but I think that's unrelated to this PR as I don't see it in the develop branch either. Or on the deployed editor.

Do you know what the behaviour of the Timer is and I'll try and fix it....!

Oh thanks for pointing that out! It's the Timer.jsx component that renders in the Editor.jsx component. Since this is unrelated to this PR, I'll open a new issue so it can be merged in a separate PR :)

@catarak
Copy link
Member

catarak commented Sep 21, 2020

Just fixed the Timer issue with #1617 :)

Copy link
Member

@catarak catarak left a comment

Choose a reason for hiding this comment

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

okay this is now working as expected for me 😄 so rad!

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.

Translations for Dates

3 participants