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

Allow settings to be deleted from a snapshot #17

Closed
westonruter opened this Issue Apr 26, 2016 · 5 comments

Comments

Projects
None yet
3 participants
@westonruter
Copy link
Contributor

westonruter commented Apr 26, 2016

Given a UI for listing out the available snapshots in the WP Admin (#12), if opening a snapshot in its edit post screen (see also #16), there could be a listing of the settings in that snapshot along with checkboxes that when checked, a button could be clicked to remove them from the snapshot.

@westonruter

This comment has been minimized.

Copy link
Contributor Author

westonruter commented Jun 12, 2016

Likewise, when calling $snapshot->set( $setting_id, null ) this should have the effect of removing the setting from the snapshot.

@valendesigns

This comment has been minimized.

Copy link
Member

valendesigns commented Jun 12, 2016

That isn't really needed in wp-admin. I envisioned creating a backbone collection of the setting IDs and using buttons to remove and undo. The buttons would modified the collection and dynamically update a hidden input. That way when "Save Draft" is clicked the new content is updated without those IDs.

@westonruter

This comment has been minimized.

Copy link
Contributor Author

westonruter commented Jun 12, 2016

@valendesigns OK, so you're thinking to not do this the WP admin edit post screen, but rather inside the Customizer itself? I think this ties into that idea we had about providing a UI to list out what the current pending changes are in the Customizer. When loading the Customizer with a snapshot, we'd need to grab all of the setting values prior to calling preview and then export them to JS; we likewise have the original setting values in wp.customize.settings.settings. Then in the UI for showing the dirty settings, all that we'd need to do is have a button listed with each setting that says “revert” and the behavior would be to override the setting's value to be whatever the original/non-snapshotted value was, and then set _dirty to false.

The trick then would be if the user updates the snapshot, we'd also need to send a list of all of the settings that have been reverted so that when the snapshot is saved, those settings can be removed from it. This was what I was referring to with null. If reverted settings persist in the settings that are sent to the server when updating the snapshot, but instead of being the actual (when they are dirty) value we send null instead, this can be a way to indicate that the setting should be dropped from the snapshot.

@valendesigns

This comment has been minimized.

Copy link
Member

valendesigns commented Jun 12, 2016

No, I think you misunderstood me, we're talking about two different pieces of the puzzle. I'm saying that we don't need to bootstrap the Customizer in wp-admin and use the CustomizeSnapshots::set() method to remove settings by supplying a null value. Instead we filter the post_content during save to unset the IDs we want removed.

What you mentioned above still is valid if we want to revert settings in the Customizer, but that is low priority IMO.

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

@westonruter westonruter modified the milestones: 0.5.0, 0.6.0 Aug 6, 2016

@kienstra kienstra self-assigned this Aug 22, 2016

kienstra added a commit to kienstra/wp-customize-snapshots that referenced this issue Aug 24, 2016

Issue xwp#17 : Enable deleting settings in snapshot wp-admin/post page.
Create link 'Remove setting,' which toggles to 'Restore setting' on click.
It also sets a hidden text field for each setting to be removed.
On saving, these appear in $_REQUEST[ 'customize_snapshot_remove_settings' ].
Filter post content  on 'content_save_pre,' when this is saved.
If $_REQUEST has settings to be removed, filter them out.
Also, add simple styling of this 'Remove setting' link.
It appears red when set to 'remove,' and blue when set to 'restore.'

kienstra added a commit that referenced this issue Aug 24, 2016

Issue #17 : Enable deleting settings in snapshot wp-admin/post page.
Create link 'Remove setting,' which toggles to 'Restore setting' on click.
It also sets a hidden text field for each setting to be removed.
On saving, these appear in [ 'customize_snapshot_remove_settings' ].
Filter post content  on 'content_save_pre,' when this is saved.
If  has settings to be removed, filter them out.
Also, add simple styling of this 'Remove setting' link.
It appears red when setto 'remove,' and blue when set to 'restore.'

kienstra added a commit that referenced this issue Aug 24, 2016

@kienstra

This comment has been minimized.

Copy link
Contributor

kienstra commented Aug 24, 2016

Pull Request Opened

Hi @westonruter,
Could you please review this pull request?

kienstra added a commit that referenced this issue Aug 24, 2016

kienstra added a commit that referenced this issue Aug 24, 2016

Issue #17 : Fix failing unit test: property of non-object error.
Before evaluating ->post_type, pass it to isset().
Short-circuit the conditional if it's not set.

kienstra added a commit that referenced this issue Sep 3, 2016

Issue #17 : Refactor customize-snapshots-admin.js methods.
Instead of attaching methods to 'on' event handler,
Attach them to module-scope components object.
Haven't yet implemented a way to export them, as Weston suggested.

kienstra added a commit that referenced this issue Sep 3, 2016

Issue #17 : Change logic for toggling link text.
Using Weston's snippet, output link title in PHP, with localization.
And attach localized text 'Restore settting' to the link's data-text-restore attribute.
On clicking the link, call method changeLinkText.
This toggles the title to the other value stored in the data attribute.
And stores the old title in the data attribute, for the next time it's clicked.

kienstra added a commit that referenced this issue Sep 3, 2016

Issue #17 : Remove need for object brace notation.
At Weston's suggestion, pass second argument to json_decode.
This will then return an array, instead of an object.
So access the array value with $data[ $setting_id ].
Instead of previous object access with $data->{ $setting_id }.

kienstra added a commit that referenced this issue Sep 3, 2016

Issue #17 : Use $content passed to filter, instead of $post->post_con…
…tent.

Also, wp_unslash() the $content.
Otherwise, the json_decode function will return null.

kienstra added a commit that referenced this issue Sep 4, 2016

Issue #17 : Add is_array() to $should_filter_content conditional.
Before iterating through the settings to unset,
Ensure that the settings are in an array, as expected.

kienstra added a commit that referenced this issue Sep 4, 2016

westonruter added a commit that referenced this issue Oct 20, 2016

Merge pull request #84 from xwp/feature/allow-deleting-settings
Issue #17 : Enable deleting settings in snapshot wp-admin/post page.

@westonruter westonruter modified the milestones: Next Major Release, 0.6.0 Feb 6, 2017

@westonruter westonruter modified the milestones: 0.6.0, Next Major Release Jul 6, 2017

@westonruter westonruter closed this Jul 6, 2017

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.