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

Clear up distinction between previewing vs saving values in a snapshot #26

Closed
westonruter opened this issue Apr 28, 2016 · 0 comments

Comments

Projects
None yet
2 participants
@westonruter
Copy link
Contributor

commented Apr 28, 2016

There seems to be somewhat of a gray area in Customize Snapshots, and in the Customizer in general for previewing vs saving values. In the Customize Snapshots plugin the can_preview method is used for both of these conditions, it seems going for the save check by seeing if is_admin(). I think perhaps they should be separated out a but more to make it more black and white?

diff --git a/php/class-customize-snapshot-manager.php b/php/class-customize-snapshot-manager.php
index dab955d..0c1208d 100644
--- a/php/class-customize-snapshot-manager.php
+++ b/php/class-customize-snapshot-manager.php
@@ -305,7 +305,7 @@ class Customize_Snapshot_Manager {
        }

        foreach ( $this->customize_manager->settings() as $setting ) {
-           if ( $this->can_preview( $setting, $this->unsanitized_snapshot_post_data ) ) {
+           if ( $this->can_preview( $setting, $this->unsanitized_snapshot_post_data ) && $setting->check_capabilities() ) {
                $post_data = $this->unsanitized_snapshot_post_data[ $setting->id ];
                $this->snapshot->set( $setting, $post_data['value'], $post_data['dirty'] );
            }
@@ -518,9 +518,6 @@ class Customize_Snapshot_Manager {
        if ( ! ( $setting instanceof \WP_Customize_Setting ) ) {
            return false;
        }
-       if ( ! $setting->check_capabilities() && is_admin() ) {
-           return false;
-       }
        if ( ! array_key_exists( $setting->id, $values ) ) {
            return false;
        }
@@ -538,7 +535,7 @@ class Customize_Snapshot_Manager {
            $this->customize_manager->add_dynamic_settings( array_keys( $values ) );

            foreach ( $this->snapshot->settings() as $setting ) {
-               if ( $this->can_preview( $setting, $values ) ) {
+               if ( $this->can_preview( $setting, $values ) && $setting->check_capabilities() ) {
                    $this->customize_manager->set_post_value( $setting->id, $values[ $setting->id ] );
                }
            }

I think maybe the can_preview method should be removed entirely?

diff --git a/php/class-customize-snapshot-manager.php b/php/class-customize-snapshot-manager.php
index dab955d..6ef1a7d 100644
--- a/php/class-customize-snapshot-manager.php
+++ b/php/class-customize-snapshot-manager.php
@@ -305,7 +305,7 @@ class Customize_Snapshot_Manager {
        }

        foreach ( $this->customize_manager->settings() as $setting ) {
-           if ( $this->can_preview( $setting, $this->unsanitized_snapshot_post_data ) ) {
+           if ( array_key_exists( $setting->id, $this->unsanitized_snapshot_post_data ) && $setting->check_capabilities() ) {
                $post_data = $this->unsanitized_snapshot_post_data[ $setting->id ];
                $this->snapshot->set( $setting, $post_data['value'], $post_data['dirty'] );
            }
@@ -508,26 +508,6 @@ class Customize_Snapshot_Manager {
    }

    /**
-    * Check if the setting can be previewed.
-    *
-    * @param \WP_Customize_Setting $setting A WP_Customize_Setting derived object.
-    * @param array                 $values  All settings' values in the snapshot.
-    * @return bool
-    */
-   public function can_preview( $setting, $values ) {
-       if ( ! ( $setting instanceof \WP_Customize_Setting ) ) {
-           return false;
-       }
-       if ( ! $setting->check_capabilities() && is_admin() ) {
-           return false;
-       }
-       if ( ! array_key_exists( $setting->id, $values ) ) {
-           return false;
-       }
-       return true;
-   }
-
-   /**
     * Set the snapshot settings post value.
     */
    public function set_post_values() {
@@ -538,7 +518,7 @@ class Customize_Snapshot_Manager {
            $this->customize_manager->add_dynamic_settings( array_keys( $values ) );

            foreach ( $this->snapshot->settings() as $setting ) {
-               if ( $this->can_preview( $setting, $values ) ) {
+               if ( $setting->check_capabilities() ) {
                    $this->customize_manager->set_post_value( $setting->id, $values[ $setting->id ] );
                }
            }
@@ -566,7 +546,8 @@ class Customize_Snapshot_Manager {
                $values = $this->snapshot->values();

                foreach ( $this->snapshot->settings() as $setting ) {
-                   if ( $this->can_preview( $setting, $values ) ) {
+                   $is_dirty = array_key_exists( $setting->id, $values );
+                   if ( $is_dirty ) {
                        $setting->preview();
                        $setting->dirty = true;
                    }

@valendesigns valendesigns added this to the 0.5.0 milestone Jun 12, 2016

@westonruter westonruter referenced this issue Jun 21, 2016

Closed

Refactor #53

1 of 2 tasks complete

@westonruter westonruter referenced this issue Jul 11, 2016

Merged

Refactor #59

3 of 3 tasks complete
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.