-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Chrome: Adding the Revisions Panel #910
Conversation
da64583
to
d594a81
Compare
editor/actions.js
Outdated
@@ -96,8 +95,11 @@ export function savePost( dispatch, postId, edits ) { | |||
|
|||
export function trashPost( dispatch, postId, postType ) { | |||
new wp.api.models.Post( { id: postId } ).destroy().done( () => { | |||
window.location.href = 'edit.php?post_type=' + postType | |||
+ '&trashed=1&ids=' + postId; | |||
window.location.href = getWPAdminURL( 'edit.php', { |
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.
Could this just reuse addQueryArgs
, e.g. addQueryArgs( 'edit.php', { post_type: postType } );
?
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.
Yes probably, but I thought it should be a separate function to be able to change its implementation later (#867)
d594a81
to
d655bc3
Compare
Designwise this looks great! And great to see all the progress on the sidebar as a whole! I'm getting more and more JS errors with 405 though. This one using PUT: @nylen any tips? |
I'm merging soon if no objections? |
d655bc3
to
47e61ac
Compare
|
||
/** | ||
* WordPress dependencies | ||
* Internal dependencies | ||
*/ | ||
import { getBlockSettings, switchToBlockType } from 'blocks'; |
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.
There's both WordPress and Internal dependencies here.
Aside: We could do for a better indicator.
|
||
const newUrl = baseUrl + '?' + stringify( { | ||
...qs, | ||
const newUrl = getGutenbergURL( { |
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.
Inconsistency between Url
and URL
. I think we'll want to settle on and use consistently a recommendation for acronyms. The core guidelines do not include one. Calypso recommends strict camel-casing, but I've personally grown fonder of upper-case. I can create an issue.
*/ | ||
export function getGutenbergURL( query = {} ) { | ||
const [ baseUrl, currentQuery ] = window.location.href.split( '?' ); | ||
const qs = parseQueryString( currentQuery || '' ); |
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.
Minor: The default can be assigned in the array destructure for clarity/consistency:
const [ baseUrl, currentQuery = '' ] = window.location.href.split( '?' );
closes #853
url
utils file.