-
Notifications
You must be signed in to change notification settings - Fork 116
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
Revamp widget connector #391
Conversation
Argument was passed to wp_stream_post_insert_error action
…or the customizer
Still have work to do to make this work properly in Customizer Use a different string template mechanism which prevents args from being needlessly added to Stream meta
Fix label for movement to reverse from/to sidebars Use new tpl vars for summaries
Eliminate warning related
Fix typo in logging deletions
Conflicts: connectors/widgets.php
} else { | ||
// Neither a name nor a title are available, so use the sidebar ID | ||
$message = __( '{widget_id} widget updated', 'stream' ); | ||
} |
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 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.
@westonruter Yeah it would be ideal to put the sidebar name in these summaries again, also we should use _x()
to give context to the translator.
Can you help me understand the differences between $widget_id_base
, $widget_id
and $name
? I'm a little confused.
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.
The $name
is the kind of widget that it is, e.g. Text, RSS, Categories, etc. Ideally you'd want to see both the name and the title in the stream.
If the $widget_id
is text-123
then the $widget_id_base
is text
, so just the ID without the number attached to it.
Conflicts: connectors/widgets.php
* Point to expirimental PHPCS and WPCS for rulset subsets * Introduce config file for managing Travis before_script environment * Remove phpcs.ruleset.xml since we're referencing a core ruleset See xwp/wp-dev-lib#30
Conflicts: bin/.travis.yml includes/admin.php phpcs.ruleset.xml
Conflicts: connectors/widgets.php
Closing this PR until more progress can be made in the near future. |
What was outstanding? |
@westonruter I just saw that there were a handful of connectors/widgets.php#L103-L104 |
global $wp_registered_sidebars; | ||
|
||
if ( array_key_exists( $sidebar, $wp_registered_sidebars ) ) { | ||
$links[ __( 'Edit Widget Area', 'stream' ) ] = admin_url( 'widgets.php#' . $sidebar ); // xss ok (@todo fix WPCS rule) | ||
} | ||
// @todo Also old_sidebar_id and new_sidebar_id | ||
// @todo Add Edit Widget link |
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.
These are optional nice to haves. Future enhancements.
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.
So then I guess #521 should be reopened as well.
Revamp widget connector
/five @westonruter! |
Resolves (in part) #308
This adds support for tracking widget changes in the customizer, improves the logic for tracking changes on the widgets admin page, and allows tracking of widget changes no matter how they are made (e.g. via WP-CLI).
New features:
sidebar
Stream meta has been renamed tosidebar_id
to correspond withwidget_id
. No migration script to renamestream
tostream_id
has been included.Todo:
widget_id
ever be added as a context?Add database migrations for any changed stream meta (e.g.sidebar
=>sidebar_id
)Add widget edit link, in addition to the widget area edit link.(See Add widget edit link in addition to the widget area edit link #521)