Skip to content

Sync: Sidebar changes#6909

Merged
enejb merged 6 commits intomasterfrom
add/sync-widget-settings
Apr 6, 2017
Merged

Sync: Sidebar changes#6909
enejb merged 6 commits intomasterfrom
add/sync-widget-settings

Conversation

@enejb
Copy link
Copy Markdown
Member

@enejb enejb commented Apr 4, 2017

This PR lets us sync changes as they happen to the sidebar.

Currently we don't have enough info that will let us see what is happening to the sidebar.

Changes proposed in this Pull Request:

  • Add actions that get fired when the widgets get added, removed and reorder a sidebar.
  • Also lets us know when we move a widget to the in active widgets as well as when we

Testing instructions:

  • Do the test pass?
  • Does it make do the actions get synced on the .com side as expected?

This PR lets us sync changes as they happen to the sidebar.
@enejb enejb requested review from gititon and lezama April 4, 2017 21:36
@enejb enejb added [Package] Sync [Status] Needs Review This PR is ready for review. labels Apr 4, 2017
add_action( 'jetpack_sync_current_theme_support', $callable );

// Sidebar updates.
add_action( "update_option_sidebars_widgets", array( $this, 'sync_sidebar_widgets_actions' ), 10, 2 );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could use single quotes

foreach( $added_widgets as $added_widget ) {
$moved_to_sidebar[] = $added_widget;
/**
* Helps Sync log that a widget got add
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"added"?

// lets check if we didn't move the widget to in_active_widgets

if ( isset( $new_value['wp_inactive_widgets'] ) && ! in_array( $removed_widget, $new_value['wp_inactive_widgets'] ) ) {
echo "remove....";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

echo :)

@lezama
Copy link
Copy Markdown
Contributor

lezama commented Apr 5, 2017

there are some minor coding standards issues that we should fix before merging

Copy link
Copy Markdown
Member

@thingalon thingalon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Success; I see the new events rolling through when I force my wpcom sandbox to dump Jetpack Sync events: :-)

mark_ wpdev_thingalon_dev_dfw_wordpress_com___home_wpcom_public_html ssh_wpdev bash _150x50

@thingalon thingalon added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Apr 6, 2017
return array( $this->get_theme_support_info() );
}

function sync_sidebar_widgets_actions( $old_value, $new_value ) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't a blocking comment, but the sync_sidebar_widgets_actions() method is handling a number of widget action types. Consider handling each in a small method of its own that is called from sync_sidebar_widgets_actions().

$this->assertEquals( $local_value, $this->server_replica_storage->get_option( 'theme_mods_' . $this->theme ) );
}

public function test_widgets_changes_get_synced() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't a blocking comment, but consider different tests for the different widget sync event types.

@enejb enejb merged commit 24ed093 into master Apr 6, 2017
@enejb enejb deleted the add/sync-widget-settings branch April 6, 2017 22:51
@matticbot matticbot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Apr 6, 2017
jeherve added a commit that referenced this pull request Apr 24, 2017
eliorivero pushed a commit that referenced this pull request Apr 25, 2017
* Changelog: initial commit for 4.9 release.

* Changelog: add #6929

* Changelog: move old changelogs to changelog.txt

* Readme: restore deleted release post link.

The post is now live.

* Changelog: add #6853

* Changelog: add #6856

* Changelog: add #6857

* Changelog: add #6884

* Changelog: add #6885

* Changelog: add #6892

* Changelog: add #6894

* Changelog: add #6898

* Changelog: add #6899

* Changelog: add #6900

* Changelog: add #6909

* Changelog: add #6927

* Changelog: add #6947

* Chagelog: add #6958

* Changelog: add #6961

* Changelog: add #6963

* Changelog: add #6965

* Changelog: add #6986

* Changelog: add #7000

* Changelog: add #7013

* Changelog: add #7015

* Changelog: add #7019

* Changelog: add #7028

* Changelog: add #6998

* Changelog: add #6999

* Changelog: add #7044

* Changelog: add #6881

* Changelog: add #6922

* Changelog: add #6940

* Changelog: add #6962

* Changelog: add #6942

* Changelog: add #6959

* Changelog: add #7018

* Changelog: add #6948

* Changelog: add #6657

* Changelog: add #7030

* Changelog: add #7048

* Changelog: add #7031

* Changelog: add #6990

* Changelog: add #6957

* Changelog: add #7027
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants