-
Notifications
You must be signed in to change notification settings - Fork 651
Initial JS Refactor for V2 #1374
Conversation
wp-api.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tlovett1 avatar_url should now return an object
|
@tlovett1 Can you merge @WP-API/amigos How should we approach this? I was thinking we could merge Taylor's initial pass and then iterate from there. Any objections? |
I'm fine with this. Before we merge though, can one of you open another issue with a checklist of the remaining changes / improvements needing to be made? |
|
@rachelbaker merged in develop. |
|
Any updates? @rachelbaker @rmccue |
|
@tlovett1 Is this also updated over at https://github.com/WP-API/client-js ? I'd like to ensure we maintain that repo as the canonical source for the JS for now, given that it'll probably need further updating too. Happy to merge, would like a quick review from @mattheu or @kadamwhite first. |
|
@rmccue The client js repo is updated now. |
wp-api.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should still be X-WP-Nonce, PHP just internally mangles the name.
|
Will take a look a bit later today , thanks for tagging me Ryan |
wp-api.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ought to go inside the IIFE—this will impact other included scripts if somebody uses a plugin to concatenate this with any of their other JS dependencies.
wp-api.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Syntax error: should be 'undefined' === typeof response.author, as it is below. Although since we have Underscore I'd probably _.isUndefined()
|
Looks good! Found one syntax error and a couple areas where we could clarify the code or simplify it with Underscore methods, but that's about it |
|
@rmccue worked in most of @kadamwhite's feedback (thanks!). Let me know if you need anything else. |
|
looks good |
Initial JS Refactor for V2
|
Merged #1374 Thank you @tlovett1 and @kadamwhite |
|
From #1350: Any specific updates on issues with |
|
@Jany-M, the behavior of the |
|
@kadamwhite using |
JS has been refactored, cleaned up, and prepared for V2. Right now there are some holes such as:
/posts/1/terms/post_tagThere is also more cleanup to do, especially surrounding documentation.
All feedback is welcome :)
Fixes #1320