Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 8 years ago

#27112 closed task (blessed) (fixed)

Add widget management to the customizer

Reported by: nacin's profile nacin Owned by:
Milestone: 3.9 Priority: normal
Severity: normal Version:
Component: Widgets Keywords:
Focuses: Cc:

Description

As decided last week, we're going to give the Widget Customizer plugin a go in core. This ticket is to track its initial merge, which ocean90 and I are heading up.

We're going to use the "Widgets" component for this.

Attachments (6)

option.php.diff (556 bytes) - added by westonruter 11 years ago.
Add pre_update_option filter to update_option()
27112.diff (156.1 KB) - added by nacin 11 years ago.
Screenshot 2014-03-11 13.11.49.png (36.6 KB) - added by victortihai 11 years ago.
27112.jetpack-widget-visibility-compat.patch (1.2 KB) - added by westonruter 11 years ago.
Enqueue customize-widget.js in footer for compat with Jetpack Widget Visibility, and remove overrides for that specific plugin. Patch is also located on a branch on GitHub: https://github.com/x-team/wordpress-develop/compare/master...customizer-widget-visibility A corresponding Jetpack patch has also been submitted via https://github.com/Automattic/jetpack/pull/349
27112.2.diff (498 bytes) - added by kovshenin 11 years ago.
27112.no-options-transaction.diff (7.8 KB) - added by westonruter 11 years ago.
Add pre_update_option filter, replace Options_Transaction with Option_Update_Capture. https://github.com/x-team/wordpress-develop/pull/1/files

Download all attachments as: .zip

Change History (60)

#1 follow-up: @nacin
11 years ago

Some particular things we've noticed so far:

  • Need to merge the widget icons into Dashicons (melchoyce, michael arestad)
  • filter_*(), exceptions, and other things we don't use in core should be replaced
  • Twenty Thirteen and Fourteen need the JS added that the plugin bundles.

Where things should be merged:

  • widget-customizer.css should be merged into customize-controls.css.
  • widget-customizer.php should become class-wp-customize-widgets.php.
  • The two controls should be added into class-wp-customize-control.php.
  • widget-customizer.js is a ton of code, so let's maybe keep it separate from customize-control.js for now.
  • Does widget-customizer-preview.js share a lot of code with widget-customizer.js?
  • widget-customizer-preview.css should just be some CSS injected into wp_head. (It also needs better styling.)
  • What's liveFilter for? Is it needed? Can we do our own thing? The Themes page already has live searching.
  • I'm curious whether the options transaction component is necessary. We built the customizer originally by simply storing values and passing them through option_* filters on the fly. I've looked at this a few times now, and I'm not sure what I'm missing?
Last edited 11 years ago by ocean90 (previous) (diff)

#2 in reply to: ↑ 1 @shaunandrews
11 years ago

Replying to nacin:

  • What's liveFilter for? Is it needed? Can we do our own thing? The Themes page already has live searching.

Its what powers the widget search filter. If it can be replaced with something simpler, or similar to THX, then by all means...

#3 @westonruter
11 years ago

Important quick note: we have the 0.15 release for the plugin queued for releasing today. This has a bunch of fixes and features, including support for wide widget controls and live previews for widget changes as you type them (no need to click Update). Here's the current changelog: https://github.com/x-team/wp-widget-customizer/compare/0.14...master

#4 @westonruter
11 years ago

Version 0.15 has been released: https://github.com/x-team/wp-widget-customizer/compare/0.14...0.15

  • Add support for wide widget controls by sliding them out horizontally over the preview. Props westonruter.
  • Eliminate Update button and so preview updates with each input change for widgets that support live previews. Props westonruter.
  • Make widget form controls more compact on smaller screen resolutions. Props michael-arestad.
  • Improve styling of widget search field. Props shaunandrews.
  • Rename "Update" button to "Apply". Props arnoesterhuizen.
  • Prevent error when initializing sidebar containing unregistered widget. Props westonruter.
  • Only show one widget form control expanded at a time. Props westonruter.
  • Eliminate use of filter_input(). Props westonruter.
  • Add live preview support for Twenty Fourteen Ephemera Widget.

#5 @westonruter
11 years ago

Does widget-customizer-preview.js share a lot of code with widget-customizer.js?

No. There are a couple little functions like widget_id_to_setting_id which are duplicated, but otherwise they are unique. But I was thinking of creating a widget-customizer-base.js that had these few common functions, and then make it a dependency for widget-customizer-panel.js and widget-customizer-preview.js.

@westonruter
11 years ago

Add pre_update_option filter to update_option()

#6 follow-up: @westonruter
11 years ago

I'm curious whether the options transaction component is necessary. We built the customizer originally by simply storing values and passing them through option_* filters on the fly. I've looked at this a few times now, and I'm not sure what I'm missing?

As per our wordpress-dev chat today, I've reworked the Options Transactions into an Option Capture functionality. With the patch above applied to core, the following change to the plugin can be applied: https://github.com/x-team/wp-widget-customizer/compare/eliminate-options-transactions

Note that it does not currently support add_option, for which there may be instances of a single widget declaring the option it saves into up front.

#7 in reply to: ↑ 6 @nacin
11 years ago

Replying to westonruter:

Note that it does not currently support add_option, for which there may be instances of a single widget declaring the option it saves into up front.

On the other hand: since these are single-instance widgets, if the widget is already in use, then capturing update_option() is enough. If the widget is not in use, we don't care strongly if add_option() gets used. I see two potential side effects: if an option is conditionally (and oddly) initialized (low severity), and that it has the potential to mess with future defaults (low severity but probably pretty weird if you encountered it).

#8 @nacin
11 years ago

That said, as long as we tracked and deleted add_option() calls, we could delete them after. That would solve our future defaults issue and would allow for a rollback for the conditional initialization issue. It would be muuch simpler than the older transaction methods, at least.

@nacin
11 years ago

This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.


11 years ago

#10 @westonruter
11 years ago

We discussed on IRC the removal of the current partial preview refreshing of widgets in favor of generalizing the functionality so that any customize control can add support for doing so; this partial preview refresh is key to keeping things DRY by eliminating the current requirement that all setting updates via postMessage transport re-implement all rendering logic in JavaScript for components that are normally written out in PHP—but even when the render logic is attempted to be written in JS, it will never be able to account for any filters added in PHP.

For instance, with such partial preview refreshes, the Navigation Menu control needn't do a full refresh of the preview: the menu's container could be targeted for a partial refresh, where the new menu gets rendered via an Ajax request and the response replaces the old menu.

@nacin: I've created a branch for Widget Customizer and have ripped out the partial preview refreshing: https://github.com/x-team/wp-widget-customizer/compare/without-partial-previews

Hopefully this is helpful to you. Let me know if there's anything I can do to help with the merge.

#11 @westonruter
11 years ago

A few related tickets, some of which are dependencies:

  • #26633: Customizer form submission prevention impairs accessibility of links in customizer controls
  • #26061: Customizer settings with non-scalar values incorrectly trigger as changed
  • #26569: URLs exported to JavaScript in Customizer settings get double-encoded
  • #25368: Add temp hooks for Widgets UI Refresh plugin-as-feature
  • #25419: Add support to widgets for icons and screenshots

#12 @nacin
11 years ago

In 27419:

Add widget management to the customizer.

This brings in the Widget Customizer plugin: https://wordpress.org/plugins/widget-customizer/.

props westonruter, shaunandrews, michael-arestad, johnregan3, akeda, topher1kenobe, topquarky, bobbravo2, ricardocorreia. And for good measure, props westonruter.
see #27112.

#13 follow-ups: @westonruter
11 years ago

See bug #27291 related to this merge.

I also saw what I believe to be an unrelated issue: if I make a change to a widget, say its title, leaving focus in the input and then immediately click "Save & Publish" the setting does not get saved, even though I can see it in the preview. If I first click out of the input (blur) and then click "Save & Publish", the setting gets updated as expected. It sticks. I'll do more investigation.

#14 follow-ups: @Frumph
11 years ago

This implementation for themes with a lot of widget sidebars adds too much to the customizer at the top of the list, if it can be prioritized to show after the rest are added that would be better otherwise it's a lot of scrolling to get to what people want to really use.

Last edited 11 years ago by Frumph (previous) (diff)

#15 in reply to: ↑ 14 @ScottSmith
11 years ago

Replying to Frumph:

This implementation for themes with a lot of widget sidebars adds too much to the customizer at the top of the list, if it can be prioritized to show after the rest are added that would be better otherwise it's a lot of scrolling to get to what people want to really use.

It makes more sense to me if the widgets come just before the menu section, considering that the Appearance Menu orders them Customize, Widgets, then Menus.

#16 in reply to: ↑ 14 ; follow-up: @nacin
11 years ago

Replying to Frumph:

This implementation for themes with a lot of widget sidebars adds too much to the customizer at the top of the list, if it can be prioritized to show after the rest are added that would be better otherwise it's a lot of scrolling to get to what people want to really use.

They should be moved to the bottom; it just didn't happen in the initial commit.

Please upload only SFW and appropriate screenshots, please.

#17 in reply to: ↑ 13 @DrewAPicture
11 years ago

Replying to westonruter:

I also saw what I believe to be an unrelated issue: if I make a change to a widget, say its title, leaving focus in the input and then immediately click "Save & Publish" the setting does not get saved, even though I can see it in the preview. If I first click out of the input (blur) and then click "Save & Publish", the setting gets updated as expected. It sticks. I'll do more investigation.

See #27338

#18 @nacin
11 years ago

In 27492:

Add widget customizer tests missing from [27419]. see #27112.

#19 @nacin
11 years ago

In 27493:

Revert [27492]. see #27112.

#20 follow-up: @ScottSmith
11 years ago

Live widget updates don't seem to be functioning in Twenty Fourteen as of beta 1. The page appears to be refreshing after a delay rather than updating live.

#21 in reply to: ↑ 20 ; follow-up: @westonruter
11 years ago

Replying to ScottSmith:

Live widget updates don't seem to be functioning in Twenty Fourteen as of beta 1. The page appears to be refreshing after a delay rather than updating live.

This is expected. All widget changes now trigger a preview refresh. See comment above: https://core.trac.wordpress.org/ticket/27112?replyto=20#comment:10

For now, if your server is slow, or your site is very front-end heavy, you will notice a preview refresh.

This ticket was mentioned in IRC in #wordpress-dev by DrewAPicture. View the logs.


11 years ago

#23 @westonruter
11 years ago

Filed related tickets:

  • #27355 Customizer: Add framework for partial preview refreshes
  • #27356 Widget Customizer: Add support for RTL
  • #27358 Widget Customizer: Improve behavior of widget highlighting

#24 follow-up: @puneetsahalot
11 years ago

Tested it with Twenty Twelve and Twenty Fourteen by adding add_theme_support('widget-customizer');
but that didn't work.

#25 in reply to: ↑ 24 ; follow-up: @westonruter
11 years ago

Replying to puneetsahalot:

Tested it with Twenty Twelve and Twenty Fourteen by adding add_theme_support('widget-customizer');
but that didn't work.

What specifically does not work? The add_theme_support( 'widget-customizer' ) call is not used by the Widget Customizer merged into core. It was used only by the plugin, and the features that this would have enabled have been captured in #27355.

#26 @victortihai
11 years ago

Hi,

I think will be fine to add the possibility to order widgets or to keep the same order like on widgets section. The widgets should be after the navigation section.

#27 in reply to: ↑ 25 ; follow-up: @puneetsahalot
11 years ago

Replying to westonruter:

Replying to puneetsahalot:

Tested it with Twenty Twelve and Twenty Fourteen by adding add_theme_support('widget-customizer');
but that didn't work.

What specifically does not work? The add_theme_support( 'widget-customizer' ) call is not used by the Widget Customizer merged into core. It was used only by the plugin, and the features that this would have enabled have been captured in #27355.

Sorry. Must have been an issue with my WP version.
It works fine now.

@victortihai : I agree, widget order should be same as on widgets page.

+ There should be a way to access widgets under "inactive widgets"

#28 follow-up: @puneetsahalot
11 years ago

Page reloads while adding content to a widget and then widget doesn't expand.

E.g. : added a text widget and while I was typing (pretty fast), the page reloaded and here's how it appears when I expand the widget
http://iampuneet.com/wp-content/uploads/2014/03/text-widget-customizer.png

Next, I tried to test Archives Widget, while I was adding a title for the widget the same thing happened.
http://iampuneet.com/wp-content/uploads/2014/03/archives-widget-customizer.png

#29 in reply to: ↑ 28 ; follow-up: @westonruter
11 years ago

Replying to puneetsahalot:

Page reloads while adding content to a widget and then widget doesn't expand.

E.g. : added a text widget and while I was typing (pretty fast), the page reloaded and here's how it appears when I expand the widget

Next, I tried to test Archives Widget, while I was adding a title for the widget the same thing happened.

This issue was reported in #27291. I have a patch there attached to the ticket which resolved the issue related to adding new widgets to the customizer.

#30 in reply to: ↑ 29 @puneetsahalot
11 years ago

Replying to westonruter:

Replying to puneetsahalot:

Page reloads while adding content to a widget and then widget doesn't expand.

E.g. : added a text widget and while I was typing (pretty fast), the page reloaded and here's how it appears when I expand the widget

Next, I tried to test Archives Widget, while I was adding a title for the widget the same thing happened.

This issue was reported in #27291. I have a patch there attached to the ticket which resolved the issue related to adding new widgets to the customizer.

Thanks @westonruter
I am new to Trac. Sorry for any inconvenience caused.

#31 in reply to: ↑ 21 ; follow-up: @ScottSmith
11 years ago

Replying to westonruter:

Replying to ScottSmith:

Live widget updates don't seem to be functioning in Twenty Fourteen as of beta 1. The page appears to be refreshing after a delay rather than updating live.

This is expected. All widget changes now trigger a preview refresh. See comment above: https://core.trac.wordpress.org/ticket/27112?replyto=20#comment:10

For now, if your server is slow, or your site is very front-end heavy, you will notice a preview refresh.

Should rearranging widgets in the sidebar trigger a preview refresh? Apparently everything I do with the widget section requires the site to refresh—local and remote server alike.

#32 in reply to: ↑ 31 ; follow-up: @westonruter
11 years ago

Replying to ScottSmith:

Should rearranging widgets in the sidebar trigger a preview refresh? Apparently everything I do with the widget section requires the site to refresh—local and remote server alike.

Yes. Everything would cause a refresh of the preview currently. The refresh requirement will be addressed in #27355. But what do you mean "local and remote server alike"? Local how?

#33 in reply to: ↑ 32 @ScottSmith
11 years ago

Replying to westonruter:

Replying to ScottSmith:

Should rearranging widgets in the sidebar trigger a preview refresh? Apparently everything I do with the widget section requires the site to refresh—local and remote server alike.

Yes. Everything would cause a refresh of the preview currently. The refresh requirement will be addressed in #27355. But what do you mean "local and remote server alike"? Local how?

By "local," I meant on a MAMP server since you mentioned that a noticeable preview refresh could be because of a slow server. I wasn't saying that the preview was slow though. I was under the impression that when widgets are re-ordered the sidebar should update without a preview refresh. It's hard to find the latest information on this.

#34 follow-up: @adampickering
11 years ago

Questions regarding this.

  • If the widget area does not have any active widgets in the widget area, does it show that widget area in the customizer?
  • Say your home page template has widgets dedicated to that template page. If you are viewing another page in the customizer (i.e. blog), does it show all widget areas in the customizer. For example: the home page dedicated widget areas.

I don't think much of this could happen, based on how I believe the customizer works, but thought i'd ask.

#35 in reply to: ↑ 34 @westonruter
11 years ago

Replying to adampickering:

  • If the widget area does not have any active widgets in the widget area, does it show that widget area in the customizer?

Yes, if the widget area is used in the template being previewed (i.e. the widget area is rendered with a dynamic_sidebar() call), then the customizer section for that widget area would be displayed, even if the sidebar is empty (depending on #25368) or if all of the widgets in the sidebar are not active for the currently previewed URL. If a widget is not rendered in the template (e.g. it is turned off via Jetpack's Widget Visibility) then the widget control will have a translucent appearance. (I just noticed Widget Visibility doesn't work currently, but I see the fix.)

Replying to adampickering:

  • Say your home page template has widgets dedicated to that template page. If you are viewing another page in the customizer (i.e. blog), does it show all widget areas in the customizer. For example: the home page dedicated widget areas. I don't think much of this could happen, based on how I believe the customizer works, but thought i'd ask.

If only the front-page.php template, for example, uses a dynamic_sidebar( 'home' ) then the customizer section for that widget area will only appear in the customizer when the home page is being previewed in the customizer. Note that you can navigate around the site within the customizer, or you can navigate to an arbitrary page and then enter the customizer from that page to then open the customizer previewing that URL. As you navigate around the site in the customizer, the widget area sections will show/hide based on which widget areas are used in the template.

@westonruter
11 years ago

Enqueue customize-widget.js in footer for compat with Jetpack Widget Visibility, and remove overrides for that specific plugin. Patch is also located on a branch on GitHub: https://github.com/x-team/wordpress-develop/compare/master...customizer-widget-visibility A corresponding Jetpack patch has also been submitted via https://github.com/Automattic/jetpack/pull/349

#36 in reply to: ↑ 13 @westonruter
11 years ago

Replying to westonruter:

See bug #27291 related to this merge.

I also saw what I believe to be an unrelated issue: if I make a change to a widget, say its title, leaving focus in the input and then immediately click "Save & Publish" the setting does not get saved, even though I can see it in the preview. If I first click out of the input (blur) and then click "Save & Publish", the setting gets updated as expected. It sticks. I'll do more investigation.

@nacin: This has been addressed in #27390. @ocean90 please review, as it introduces some new plumbing to the customizer. I believe this to be a very important patch to include, as right now it is very easy for the race condition to result in a failure to save the widget settings as the user expects.

#37 @westonruter
11 years ago

By the way, for each ticket related to Widget Customizer, I have a feature branch on our GitHub clone of develop.git.wordpress.org: https://github.com/x-team/wordpress-develop/branches

Each commit (patch) added to any of these feature branches (tickets) I'm also merging into a common widget-customizer-patches branch so that the patches can be seen in relation to each other and so that conflicts can be discovered: https://github.com/x-team/wordpress-develop/compare/widget-customizer-patches

@kovshenin
11 years ago

#38 follow-up: @kovshenin
11 years ago

There's some debug cruft in customize-widgets.js, see 27112.2.diff.

#39 in reply to: ↑ 38 @westonruter
11 years ago

Replying to kovshenin:

There's some debug cruft in customize-widgets.js, see 27112.2.diff.

Debug cruft also removed in patch on #27291: https://core.trac.wordpress.org/attachment/ticket/27291/27291.2.diff

#40 @westonruter
11 years ago

Filed:

  • #27400: Widget Customizer: Add widget reordering icons to Dashicons

This ticket was mentioned in IRC in #wordpress-dev by westonruter. View the logs.


11 years ago

#42 in reply to: ↑ 16 @westonruter
11 years ago

Replying to nacin:

Replying to Frumph:

This implementation for themes with a lot of widget sidebars adds too much to the customizer at the top of the list, if it can be prioritized to show after the rest are added that would be better otherwise it's a lot of scrolling to get to what people want to really use.

They should be moved to the bottom; it just didn't happen in the initial commit.

Addressed in #27401

#43 in reply to: ↑ 27 @westonruter
11 years ago

Replying to puneetsahalot:

@victortihai : I agree, widget order should be same as on widgets page.

Addressed in #27401

Replying to puneetsahalot:

+ There should be a way to access widgets under "inactive widgets"

Addressed in #27404

This ticket was mentioned in IRC in #wordpress-dev by westonruter. View the logs.


11 years ago

@westonruter
11 years ago

Add pre_update_option filter, replace Options_Transaction with Option_Update_Capture. https://github.com/x-team/wordpress-develop/pull/1/files

#45 @ocean90
11 years ago

[27585] - Widget Customizer: Move preview script to wp_default_scripts().

#46 @ocean90
11 years ago

In 27586:

Widget Customizer: Remove special filter for Settings Revisions plugin.

see #27112.

#47 @ocean90
11 years ago

In 27588:

Widget Customizer: Remove unused vars.

see #27112.

#48 @nacin
11 years ago

In 27620:

More translation cleanups.

Affects widgets (see #27112), custom headers (see #21785), theme installer (see #27055, reverts [27614]), and some media stuff. Untranslates some complicated strings that need additional study.

see #27453.

#50 @ocean90
11 years ago

In 27824:

Widget Customizer: Remove some specific styles for Widget Visibility from Jetpack.

This hasn't worked well and is now fixed upstream.

props westonruter.
see #27112.

#51 @ocean90
11 years ago

  • Resolution set to fixed
  • Status changed from new to closed

Closing as fixed. For new issues please open new tickets.

#52 @nacin
11 years ago

In 28140:

Add missing string translation in widgets.

see #27453, #27112.

#53 @ocean90
11 years ago

#16109 was marked as a duplicate.

#54 @afercia
8 years ago

In 40569:

Customize: Fix a visual glitch on the widget control animation introduced in [40480].

Also, restores the original design intent that was meant to "compact widget-tops
on smaller laptops, but not tablets".

See #27112.
Fixes #31476.

Note: See TracTickets for help on using tickets.