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

Improve WP Admin UI #38

Merged
merged 36 commits into from Jun 10, 2016

Conversation

Projects
None yet
2 participants
@westonruter
Copy link
Contributor

commented Jun 9, 2016

No description provided.

@westonruter westonruter force-pushed the feature/wp-admin branch to 083debb Jun 9, 2016

@@ -284,7 +289,7 @@ public function create_post_type() {
*/
function setup_metaboxes() {
$id = self::POST_TYPE;
$title = __( 'Data', 'customize-snapshots' );
$title = __( 'Data', 'customize-snapshot' );

This comment has been minimized.

Copy link
@valendesigns

valendesigns Jun 9, 2016

Member

The text domain is customize-snapshots

if ( ! empty( $allcaps['edit_theme_options'] ) ) {
$allcaps['customize_publish'] = true;
}
// Grant all customize snapshot caps which weren't explicitly disallowed to users who can customize.

This comment has been minimized.

Copy link
@valendesigns

valendesigns Jun 9, 2016

Member

This is causing test failures.

This comment has been minimized.

Copy link
@westonruter

westonruter Jun 9, 2016

Author Contributor

I restored the initial cap check in ab6ca42. Tests are passing now.

@@ -494,9 +502,6 @@ public function update_snapshot() {
if ( ! check_ajax_referer( self::AJAX_ACTION, 'nonce', false ) ) {
status_header( 400 );
wp_send_json_error( 'bad_nonce' );
} elseif ( ! current_user_can( 'customize' ) ) {

This comment has been minimized.

Copy link
@valendesigns

valendesigns Jun 9, 2016

Member

Actually this is what is causing the test failures. Not sure why you want to remove this?

This comment has been minimized.

Copy link
@westonruter

westonruter Jun 9, 2016

Author Contributor

@valendesigns because I added it below instead. The create_posts cap is now mapped to do_not_allow in the post type so as to hide the “Add New” links for snapshots in the admin. So then we have to use customize cap for this instead directly here, which would be doing below anyway.

This comment has been minimized.

Copy link
@valendesigns

valendesigns Jun 9, 2016

Member

You will never get pass this condition } elseif ( empty( $this->unsanitized_snapshot_post_data ) ) { because current_user_can( 'customize' ) will be false in Customize_Snapshot_Manager::capture_unsanitized_snapshot_post_data

This comment has been minimized.

Copy link
@westonruter

westonruter Jun 9, 2016

Author Contributor

I restored the customize cap check.

'<p><a href="%s" class="button button-secondary">%s</a></p>',
esc_url( $customize_url ),
esc_html__( 'Open in Customizer', 'customize-snapshots' )
);

This comment has been minimized.

Copy link
@valendesigns

valendesigns Jun 9, 2016

Member

This commit makes me happy 😄

@valendesigns

This comment has been minimized.

Copy link
Member

commented Jun 9, 2016

@westonruter When you publish the snapshot all the settings disappear from the metabox

@valendesigns

This comment has been minimized.

Copy link
Member

commented Jun 9, 2016

Oh they are hidden after published. So we need to remove the checkbox toggle and hidden attribute for published snapshots so all the settings display

@westonruter

This comment has been minimized.

Copy link
Contributor Author

commented Jun 9, 2016

@valendesigns I think the problem is that all of the settings get sent with dirty: false at the moment of Publish. We should preserve the original dirty state.

@westonruter

This comment has been minimized.

Copy link
Contributor Author

commented Jun 9, 2016

@valendesigns ok, ready for you to review.

@valendesigns

This comment has been minimized.

Copy link
Member

commented Jun 9, 2016

@westonruter I think we should hide the "Save Draft" button or change the customize button to "Edit" so it fits in the space. Thoughts?

@valendesigns

This comment has been minimized.

Copy link
Member

commented Jun 9, 2016

We also need to handle the publish button so the Snapshot updates the front-end. Otherwise scheduling will not work.

@valendesigns

This comment has been minimized.

Copy link
Member

commented Jun 9, 2016

@westonruter An issue with the updated state replacement JS is that the ? is being left behind when there are no params in the url. It should check that there are params and remove the question mark if there aren't any.

@westonruter

This comment has been minimized.

Copy link
Contributor Author

commented Jun 9, 2016

@valendesigns

I think we should hide the "Save Draft" button or change the customize button to "Edit" so it fits in the space. Thoughts? We also need to handle the publish button so the Snapshot updates the front-end. Otherwise scheduling will not work.

Actually, perhaps we should just hide the publish box entirely. This would be what #15 would implement, and at that time, the publish button could be restored.

@westonruter

This comment has been minimized.

Copy link
Contributor Author

commented Jun 9, 2016

I think I know what to do. We can just remove the publish_posts cap for now and it will just work 😄

@westonruter

This comment has been minimized.

Copy link
Contributor Author

commented Jun 10, 2016

@valendesigns oh, and you know what? Now with b3ea60a it would be sweet if when a user lacks customize_publish capability, in addition to a Save button there could be a “Submit” button that serves the same purpose as the “Submit for Review” button when a user wants to submit a post as pending status:

image

@valendesigns

This comment has been minimized.

Copy link
Member

commented Jun 10, 2016

So it makes the publish button say submit for review?

@westonruter

This comment has been minimized.

Copy link
Contributor Author

commented Jun 10, 2016

@westonruter

This comment has been minimized.

Copy link
Contributor Author

commented Jun 10, 2016

@valendesigns I merged feature/submit-for-review into this branch. I'm feeling really good about it now. I think I addressed all of your concerns.

@valendesigns valendesigns merged commit 181317a into release/0.4.0 Jun 10, 2016

@valendesigns valendesigns deleted the feature/wp-admin branch Jun 11, 2016

@westonruter westonruter modified the milestone: 0.4.0 Jun 12, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.