#17979 closed task (blessed) (fixed)
Avoid losing widgets when switching themes
Reported by: | lancewillett | Owned by: | ryan |
---|---|---|---|
Milestone: | 3.3 | Priority: | high |
Severity: | normal | Version: | 2.9 |
Component: | Widgets | Keywords: | has-patch needs-testing |
Focuses: | Cc: |
Description (last modified by )
Symptom: when switching themes all widgets are moved to "Inactive Widgets" area, leaving an empty sidebar.
This happens when:
- New theme has a different number of registered sidebars than old theme.
- New theme and old theme have the same number of registered sidebars, but the ID value for the sidebar is different between the two.
Widgets should never be put into Inactive if the new theme has a least one sidebar registered. The ID values for registered sidebars in themes should be ignored and not used for comparison.
Here are three files that contain the important pieces:
switch_theme()
in wp-includes/theme.phpretrieve_widgets()
in wp-admin/widgets.phpwp_get_sidebars_widgets()
in wp-includes/widgets.php
Possibly related to #9695, #9770, #10092, #10300, #10440, #11160.
Attachments (21)
Change History (146)
#3
@
13 years ago
When we changed the widgets screen in the admin couple of years ago, we left behind quite a bit of "compat" code for converting from the old format to the new and for supporting all existing plugins at the time.
We should revisit the widgets back-end in 3.3 and remove all that compat code together with improving the way sidebars are registered by themes and the overall handling of widget areas.
#4
@
13 years ago
- Keywords 3.3-early added; dev-feedback removed
- Milestone changed from Awaiting Review to Future Release
- Priority changed from normal to high
- Severity changed from major to normal
- Type changed from defect (bug) to enhancement
- Version changed from 3.2 to 2.9
#6
follow-up:
↓ 9
@
13 years ago
- Cc aaroncampbell added
Lance - The only thing I'd change in your proposed solution is the second one:
- If number of sidebars are not the same, move widget for first X sidebars directly across and put all widgets from the other sidebars into the first registered sidebar.
Basically if your old theme has five sidebars and the new one has three, then the first sidebar would have the widgets from 1, 4, & 5, and sidebar 2 would have have the widgets from 2, and sidebar 3 would have the widgets from 3.
#7
follow-ups:
↓ 8
↓ 10
@
13 years ago
I don't think automatically moving widgets to other sidebars is useful. For example, widgets in a footer sidebar might appear on the right etc.
My proposal is to let the user map old sidebars to new. Multiple old sidebars could be mapped to the same new sidebar.
In terms of UI, one idea would be to have a dropdown of new sidebars for each old sidebar, displayed right after switching to a new theme.
#8
in reply to:
↑ 7
@
13 years ago
Replying to scribu:
I don't think automatically moving widgets to other sidebars is useful. For example, widgets in a footer sidebar might appear on the right etc.
I see the task in this ticket as an important bug fix, the mapping and other UI improvements could be made as a future improvement, or in a plugin.
Moving widgets to the sidebar in the new theme, regardless of position, is *much more* useful than dropping them all into Inactive like it does now.
#9
in reply to:
↑ 6
@
13 years ago
Replying to aaroncampbell:
Lance - The only thing I'd change in your proposed solution is the second one:
Cool, that'd be awesome.
#10
in reply to:
↑ 7
;
follow-up:
↓ 11
@
13 years ago
Replying to scribu:
I don't think automatically moving widgets to other sidebars is useful. For example, widgets in a footer sidebar might appear on the right etc.
My proposal is to let the user map old sidebars to new. Multiple old sidebars could be mapped to the same new sidebar.
In terms of UI, one idea would be to have a dropdown of new sidebars for each old sidebar, displayed right after switching to a new theme.
While I think your solution is more complete, I think there's something to be said about doing as much automatically (no user input required) as possible. The user can rearrange the widgets, but at least content won't disappear from the site.
I'm thinking we could shoot for the solution Lance proposed for 3.3 and create a plugin that could be used to perfect the UI for the latter solution. I'm really worried that a mediocre UI for this will actually be WORSE than what we currently have.
#11
in reply to:
↑ 10
@
13 years ago
Replying to aaroncampbell:
I'm thinking we could shoot for the solution Lance proposed for 3.3 and create a plugin that could be used to perfect the UI for the latter solution. I'm really worried that a mediocre UI for this will actually be WORSE than what we currently have.
My thoughts exactly.
#13
@
13 years ago
- Keywords 3.3-early removed
- Milestone changed from Future Release to 3.3
Heh, just had a really out there idea for a UI:
Make the sidebars re-orderable, in the sense that you could swipe a sidebar from under the widgets and swap it with another.
But, of course, I agree that we should do as much as possible automatically.
#14
@
13 years ago
See this as two steps solution:
- When switching themes keep the old theme widgets positions/sidebar mapping in a transient for a day (or week). This will allow us to restore the layout perfectly if the theme is switched back. A lot of users just try new themes without actually switching permanently.
- Try to map the old sidebars to the new. This is easily done for the first sidebar (as it almost always is an actual sidebar, left or right) but is problematic for the next. The right solution seems to be to map as many sidebars as possible and move the remaining widgets to sidebar 1. However that has to be done while the user is previewing the theme too so they would know if any widgets would need fixing or repositioning.
#17
@
13 years ago
The behavior I'd expect and most like to see is for each old sidebar not identically present in the new theme to be displayed as an "archived sidebar" with all the widgets still in it, the user could then drag copies of the widgets out (ideally more than one at a time) to a new sidebar area. When done the user could then delete the entire archived sidebar area when they done.
The idea being here that once someone registers a sidebar it should persist on theme change. Widgets and their settings shouldn't be lost and should remain grouped together rather than dumped into the garbage heap that "inactive widgets" seems to be.
As an extension of this the ability to manually save sets of widgets as an archived sidebar might be interesting to some as well.
#19
@
13 years ago
- Keywords ux-feedback added
We should mock up plugins that implement each of these ideas and run user testing. So let's get coding :)
#22
@
13 years ago
My attempt hasn't been going my way tonight so I thought I'd share my approach.
In retrieve_widgets() in wp-admin/widgets.php, we should be storing the inactive widgets in $sidebars_widgets['wp_inactive_widgets']
not just by the ID of the inactive widget, but group them in a nested array by the old sidebar ID. Then in the beginning of retrieve_widgets(), the check for "known-good" sidebars can be extended to search within $sidebars_widgets['wp_inactive_widgets']
to see if any match in there as well.
With that, any widget that is inactive (from ANY theme activation, not just the previous one) could be added to the current theme if it happens to have a matching sidebar ID.
Now, if we store the sidebar name (not just ID) for the inactive widget in the newly nested array approach, a UI could be made to go through all of those inactive widgets, group them nicely by sidebar name and then give the user a dropdown to select a new sidebar.
I'd love to start implementing this but I'm not sure how to approach it as a plugin and testing any code changes directly in core is cumbersome since I have to add a widget and change the theme for every refresh.
#25
follow-up:
↓ 26
@
13 years ago
The patch attached does most of what was discussed for the basic needs here. It copies widgets across to the new sidebars based on sidebar order rather than id, and it only puts things in the inactive widgets if there are fewer sidebars in the new theme than the previous.
The problem is that when the switch_theme
action is fired we don't actually have access to the sidebar list from the new theme. The simple solution would be to include the new theme's functions.php, but the problem is that any theme using TEMPLATEPATH or STYLESHEETPATH to include things (seems to be pretty common) will be trying to include them from the incorrect theme. Unfortunately fixing #18298 won't fix this until themes stop using them.
Currently the patch attached works if you switch themes and then visit the widgets page.
#26
in reply to:
↑ 25
@
13 years ago
Replying to aaroncampbell:
The problem is that when the
switch_theme
action is fired we don't actually have access to the sidebar list from the new theme...
Currently the patch attached works if you switch themes and then visit the widgets page.
Ran into the same problem when upgrading the widgets API couple of years ago. Was testing to have an option holding the current theme's name and check it on admin init. If the name has changed, run an action 'after_theme_change' and update the option. That would let us shuffle the widgets as needed.
The other part would be to make the theme preview to show the new sidebars properly (with reassigned widgets).
#28
@
13 years ago
- Keywords has-patch needs-testing added
The new patch sets an option called "theme_switched" in switch_theme()
and then checks for it on init and fixes the widgets. It also maps leftovers to sidebar 1 instead of init, so if the old theme had 4 sidebars and the new has 3 it will do this:
- Old A -> New 1
- Old B -> New 2
- Old C -> New 3
- Old D -> New 1
Westi: It still doesn't handle preview, but but it handles the actual switch pretty nicely.
#29
@
13 years ago
17979.3.diff is just like .2 but moves extra sidebars to inactive, so if the old theme had 4 sidebars and the new has 3 it will do this:
- Old A -> New 1
- Old B -> New 2
- Old C -> New 3
- Old D -> Inactive
#30
@
13 years ago
The new patch (17979.4.diff) creates additional sidebars labeled as "Orphaned Widgets". These keep the original ID from the original theme, so if you switch back to the old theme everything goes exactly where it was. If we do this and then make widgets draggable in groups it would be a big step forward.
#32
@
13 years ago
The new patch (17979.5.diff) includes everything from .4 ("Orphaned Widgets", etc). It also stores the sidebars and widget order before the theme is switched in a transient that lasts 1 week. If you switch back to the old theme within that week your widget order is restored.
Any widgets that you added after switching and before switching back go into inactive widgets. Since we are storing widget ORDER if you update a widget your updates will persist. This also means that if you delete a widget it can't be restored.
Edit: This was per discussion in the dev chat: https://irclogs.wordpress.org/chanlog.php?channel=wordpress-dev&day=2011-08-24#m305629
#33
@
13 years ago
Lance pointed out that we created "Orphaned Widget" containers even for empty sidebars, which doesn't make any sense. 17979.6.diff fixes that.
#34
@
13 years ago
Empty orphaned widget groups weren't created on theme switch, but if you emptied one it would still persist. 17979.7.diff doesn't register the sidebar for orphaned widget containers if they're empty.
#35
@
13 years ago
Here are some ideas to improve the "Orphaned Widgets" widget groups:
- Use a more descriptive title, like "Orphaned - Sidebar 1" using the ID or title from the old theme's sidebar location
- Add a description, using the old theme name plus the sidebar's ID or title
- Style the orphaned widgets with a lighter text color to show they are not visible on the front end (it could be confusing because widgets in the right column are usually visible on the site)
None of these are crucial, and can be done later. But just to keep track.
#36
@
13 years ago
Was hoping we can take care of #11160 here too. It's not 100% related but fixing it would make life easier :)
#37
@
13 years ago
Ok, so I can pass data from the old theme to the new by storing a string like "{themename} - old-sidebar-id" as the ID for the new sidebar. Unfortunately, the use of sanitize_title() in dynamic_sidebar() prevents you from using an ID that is not already sanitized (even though with the way we store them there's really no need for that). However, this patch (17979.8.diff) works around that by making the name match the id, which causes dynamic_sidebar() to not pass the id through sanitize_title(). I'm honestly not super happy with it because it's kind of hacky, but it works and lets the user know why this new widget container is there. The hack wouldn't be necessary if we removes the sanitize_title() from dynamic_sidebar() but I don't know what that would break. It looks like it's been there for 4 years [5301].
#39
@
13 years ago
#41
@
13 years ago
On the discussion of widgets. My two cents. I think that widget registration is backwards. You need to be able to create widget areas and add widgets as data that is always independant of the theme.
So without a theme you could create mygoofysidebar and add stuff to it. And then do the mapping to the sidebar requested by the theme.
I don't find the idea of auto mapping on theme switching to be that compelling.
Unless it encourages common naming.
So if I'm a theme dev I know that everyone is nameing their widgets sidebar, I can request that widget set.
<AaronDCampbell> trevogre: Put that on the ticket :) I think that mapping automatically is a good stop-gap so fix things up for now
I agree that it would be nice to have it work more like menus (make sidebars and then assign them to locations) but that needs to be the main focus of a release because it'll be a lot of code.
<trevogre>will do, I'll throw that on the ticket. I haven't dug into the widget code so I don't know how much code it would be. I think it could be fairly light wieght but it would have to pull the sidebar list from an option instead of from register sidebar calls. It could be something like the shortcode patch that I just put in.
Where it would add any sidebars that were registered and just retain all of them.
And you could add a second array to register sidebar names from the backend with a loop.
#42
@
13 years ago
Think more about the comparison to menus. Menus seem broken to me as well, but from the other direction. You need both the ability to define something explicitly in the backend and define it explicitly in the theme and have both of those things intersect. So if my theme calls for "headermenu" it puts in a header menu in the menu editor for me to add to (or to modify from its default loop). Conversely I should be able to add a custom menu and point it to override and of the registered menus. This would help to unify people under given names. Header / Sidebar / Footer etc. And make theme switching easier both for menus and for widgets.
So menus need to add the declaration features of widgets and widgets need the editing features of menus.
I think :)
#43
@
13 years ago
Lance and I talked on Skype and decided on using .7 for this release. 17979.9.diff is based on that with some copy changes, a refresh to work with current trunk, and an added "class" option for register_sidebar()
that we can use to style the orphaned sidebars differently.
#44
@
13 years ago
I'm still not sure about all the inactive sidebars. What can the users do with them? The logical thing would probably be to be able to replace an active sidebar with an inactive. However that is pretty easy to do by dragging the widgets around. Not sure it warrants the added UI complexity.
Having the widgets magically reappear when switching to the previous theme is very neat, but showing most sidebars from the previous theme as inactive at exactly the same place (when switching from theme with 5 sidebars (twenty eleven) to theme with only one sidebar) seems quite confusing even if we make them different color.
If we have to have them perhaps we should move them between the Available Widgets and Inactive Widgets and make them different color. That would at least signify the "inactive" status.
#46
follow-up:
↓ 47
@
13 years ago
Lance was working on some CSS that makes the interface more obvious, but try this to see if it makes the working more obvious:
.sidebar-orphaned .sidebar-name, .sidebar-orphaned .sidebar-description { opacity: 0.5; }
#47
in reply to:
↑ 46
@
13 years ago
Replying to aaroncampbell:
Yes, changing the look makes it better but thinking we will have to change the placement so the users don't perceive the inactive sidebars the same as the active.
#49
follow-up:
↓ 51
@
13 years ago
[17979.10.diff] does basically what's in that screenshot. It eliminates #wp_inactive_widgets in favor of .sidebar-inactive further up (so that inactive and orphaned sidebars can be handled with the same code).
#wp_inactive_widgets is still used in some javascript in /wp-admin/js/widgets(.dev).js but I'm not sure whether we just want to replace with .sidebar-inactive or if we want to include .sidebar-inactive AND .sidebar-orphaned.
#51
in reply to:
↑ 49
@
13 years ago
Replying to aaroncampbell:
Looks good imho. Tweaked it a bit more to make the orphaned sidebars not accept widgets and auto-expand to the needed height.
#52
@
13 years ago
I think this might be a bandaid. When I think about widget configurations for themes I think about wanting to set an forget them. And if I change themes and then change back I would expect that the widget configuration with the theme should stay with the theme. So nothing I do in one theme is relevant in the other but the settings are maintained.
Also, it seems important to be able to try out a widget configuration without having to take a bucket of notes on what you are doing. So it would seem extremely useful to have a post style history of your widget configuration so that you can roll back.
Maybe this could be accomplished with a widget_settings custom post type. So everytime you make a change it saves a new version of the widget settings for a given sidebar / theme / or all widget settings, in a custom post. And then put in a rollback mechanism that allows you to recover a previous set. Or better yet be able to make the configurations and use the scheduling mechanism or other logic to do a/b tests and publish different configurations.
I guess I think that anything that wordpress takes seriously needs versioning. We have versioning outside with git and subversion. We have versioning with posts/pages/custom posts. But the rest of the database sucks for versioning. You have to rely on outside solutions to do backup and recovery.
It seems to me that backup and recovery should live in the land of disaster recovery not as a tool to recover data or settings to a previous state.
So the goal of having distinct official approaches to versioning in development and production environments that cover all code and data seems to be a good one. Without relying on any external service having wordpress specific features. (Meaning you should be able to use any backup mechinism and not need a special one like vaultpress).
#55
@
13 years ago
That should fix the problem with orphaned sidebars getting removed before they're empty.
#57
follow-up:
↓ 58
@
13 years ago
Is the screenshot from aaroncampbell still up to date?
It looks a bit different by me and I don't know if it's the right behaviour:
Theme is 2011: Widget settings: http://cl.ly/1x0C102T343t1N3p1i47
After switching to 2010 theme: Widget settings: http://cl.ly/2J0E2B3a1F371b2H442q
Now switching back to 2011 brings the same settings as in first screenshot.
If I comment out the register_sidebar
for Footer Area Three I get this: http://cl.ly/2Q261U2e022Q3w0s0H2P (As you can see the title is too long too.)
#58
in reply to:
↑ 57
@
13 years ago
Replying to ocean90:
Your third screenshot seems to 404.
As for the others, there's definitely a problem. If you are running current trunk and you switch from TwentyEleven with widgets in the sidebars to TwentyTen, all the widgets should go into the sidebars instead of inactive widgets. What you're seeing appears to be what happened BEFORE patches from this ticket landed in trunk.
#60
in reply to:
↑ 59
@
13 years ago
Replying to DrewAPicture:
I reproduced the same result(s) as ocean90 on trunk.
Can you give the steps you took to reproduce?
#61
@
13 years ago
Can you give the steps you took to reproduce?
Yes, sorry.
*Running TwentyEleven, added two default widgets (Links and Meta)
*Activated TwentyTen, checked Widgets -> All sidebars are empty.
*Reactivated TwentyEleven, 2 widgets still in sidebar.
I also tried this with simple text widgets similarly to ocean90, with the same results.
EDIT: FYI, I updated trunk via svn right before I reproduced this.
#62
@
13 years ago
@ocean90, @DrewAPicture that's strange. Since [18630] sidebars should be matched one to one so switching from 2011 to 2010 should keep them exactly the same as both themes have 5 sidebars. That's how it works for me too.
Could you put some var_dump()
in retrieve_widgets()
and see what exactly is happening. (And don't forget to delete the transients so you get 'clean' behavior.)
#63
@
13 years ago
After removing all my transients and a new theme switch the old widgets automatically go into the new sidebars now.
Also 2011 has 5 sidebars, 2010 has 6. So what should happen with the widgets from the sixth sidebar after a switch from 2010 to 2011?
Maybe you can ping me in IRC?
#64
@
13 years ago
(And don't forget to delete the transients so you get 'clean' behavior.)
- Deleted the transients
- Text widgets properly moved across on theme switch
#65
@
13 years ago
I didn't check the code, but there seems to be a focus on mapping sidebars when the names are different, based upon the number of sidebars.
I think that is a mistake. Imagine there is a scenario where I want to use theme versions as a test mechanism. So I make a new version of theme x that I want to launch on date y. It has a new layout that includes "my_extra_sidebar" and removes another sidebar. So I have 2 sidebars. The new sidebar is a left sidebar and the old a right. Which should have distinct (preprepared) content.
If the seems switch simply iterates through the existing bars and shoves them in places, that is one more manual step to insure that the new theme is functioning properly. And the inverse is true, if I automaticly switch back I have to check the site to see if its functioning.
I would rather end up with nothing in that space than the wrong thing.
This is a major issue with the existance of widgets at all. They live both in database and in the theme design. So you can't use the themes version control on them and there is no version control for them in the database. So is thier convience even worth it when they seem to encourage bad practices.
What is the workflow that incorporates the use of widgets that is not considered the data version of cowboy coding. Mucking up the presentation of your data with no rollback on your production server. Who should have that level of access? If it's at the admin level I would rather build the sidebar with shortcodes in a version controled template revision. Because according to some commiters (if not all) not using version control is wrong.
Not hacking on the effort, this is a good first step. I would just agree that the ultimate focus needs to be on registering sidebars like menus. So "footer" maps to "footer" on a theme change and no widget configurations are deleted ever. Auto mapping widget bars will just create a whole different set of problems where footer widgets get stuffed into sidbars and other equally disfiguring things.
I'm not even concerned if I know in the backend what is inactive and active. That seems like extra code that is needed less than the ability to build n configurations for widgets. If you are configuring widgets you should be able to check the site to know where your changes are occuring. And know which widget areas your theme supports. If you don't know this or aren't expected to figure it out then what are you doing with the responsibilty for changing themes?
I'm new to commenting on here, so please let me know if these kinds of verbose comments are unwarrented and/or unwanted.
#66
@
13 years ago
Just saving the widgets (aaroncampbell's mockup) doesn't honestly seem any better then the current situation of dumping them into inactive widgets garbage pile. I was expecting something like this mockup where whole sidebars get saved as groups.
http://core.trac.wordpress.org/attachment/ticket/17979/orphaned-sidebar-mockup.png
#67
follow-up:
↓ 68
@
13 years ago
When installing the fresh trunk to a fresh database, I see what ocean90 mentioned on IRC:
http://cl.ly/1z2T402d170w3C3v3H3P
After a page reload, Inactive Sidebar disappears.
This is what wp_get_sidebars_widgets()
returns first time on a clean install:
[wp_inactive_widgets] => Array() [primary-widget-area] => Array( [0] => search-2 [1] => recent-posts-2 [2] => recent-comments-2 [3] => archives-2 [4] => categories-2 [5] => meta-2 ) [secondary-widget-area] => Array() [first-footer-widget-area] => Array() [second-footer-widget-area] => Array() [third-footer-widget-area] => Array() [fourth-footer-widget-area] => Array()
Looks like the defaults in wp-admin/includes/upgrade.php
were never updated for Twenty Eleven:
http://core.trac.wordpress.org/browser/tags/3.2.1/wp-admin/includes/upgrade.php#L284
17979.twenty-eleven-defaults.patch should fix this.
#68
in reply to:
↑ 67
;
follow-up:
↓ 70
@
13 years ago
Replying to SergeyBiryukov:
I think this is happening because retrieve_widgets()
is not run on the first login after new install. The fix for wp_get_widget_defaults()
will work but only for twentyeleven. If the first sidebar id is changed in twentytwelve, that will stop working again.
Perhaps better would be to run retrieve_widgets() on the first login after install (i.e. set the option 'theme_switched' to true).
Replying to trevogre:
I didn't check the code, but there seems to be a focus on mapping sidebars when the names are different, based upon the number of sidebars.
I think that is a mistake...
I would rather end up with nothing in that space than the wrong thing.
This was the workflow / assumptions of the previous widgets admin page.
This is a major issue with the existance of widgets at all...
...
What is the workflow that incorporates the use of widgets that is not considered the data version of cowboy coding...
Widgets are user controllable and optional. If designers / theme authors decide not to allow the users to intervene, they can hard-code the sidebars easily.
Replying to jb510:
I was expecting something like this mockup...
This is very similar to the current trunk except there's no yet another box around the orphan sidebars.
#69
follow-up:
↓ 72
@
13 years ago
"This was the workflow / assumptions of the previous widgets admin page. "
The assumption of the previous admin page was that your sidebar configurations were lost when you changed themes and changed back. So testing other themes removed your current configuration. When I was referring to nothing in the space. I meant nothing on the page when I changed. Not nothing in the admin. The admin needs to keep the setting for each named sidebar. Preferrably on a theme specific basis. So 2011 and 2010 can both have thier own persistant widget configuration. And when the change happens if there is no "footer" for the new theme but there is a sidebar named "footer" it automaticly copies the existing configuration.
The case in which a progressive mapping of the number of sidebars would be useful and not just disruptive would be minimal.
"Widgets are user controllable and optional. If designers / theme authors decide not to allow the users to intervene, they can hard-code the sidebars easily. "
Widgets are a production side change to website layout. The issue is not if you allow or don't allow them. It's if they work in a manner that is consistant with best practices. Not having version control and rollback of any element of your site is not consistant with best practices. So while you can make them optional, why not just make them better, so you can use them with the same confidence as you have making changes to posts and pages.
#70
in reply to:
↑ 68
@
13 years ago
Replying to azaozz:
Perhaps better would be to run retrieve_widgets() on the first login after install (i.e. set the option 'theme_switched' to true).
In that case, Inactive Widgets becomes pre-populated on the first login, which seems weird (currently it happens too, being a regression from 3.1 to 3.2).
So I guess updating sidebars_widgets
defaults when releasing a new theme is a cleaner solution.
#72
in reply to:
↑ 69
@
13 years ago
Replying to trevogre:
...The admin needs to keep the setting for each named sidebar. Preferrably on a theme specific basis. So 2011 and 2010 can both have thier own persistant widget configuration.
Having persistent widgets configs wouldn't work well. So the users can have their old widgets back if they decide to switch themes after a year or two? IMO that would be unexpected (at least) and probably considered a bug. And what happens to all new widgets that were in use before the theme change? We show them on the widgets screen as inactive? Imagine the user tries 20-30 themes for couple of years, the widgets screen will be about 10 pages long and extremely complex, quite unusable.
Widgets are a production side change to website layout... Not having version control and rollback of any element of your site is not consistant with best practices.
Rollback for individual widgets (and version control) is out of scope for this ticket. Also think there were few older tickets about that. There are also plugins that do this.
#74
follow-up:
↓ 80
@
13 years ago
As per today's dev chat 17979.12.diff switches over to theme mods instead of transients. It stores an associative array where [data] is the $sidebars_widgets array and [time] is time() from when it's stored. We currently do nothing with time() but it's there so we can use it in the future.
#75
@
13 years ago
Azaozz,
"Having persistent widget configs wouldn't work well".
I hear what you are saying, but changing themes doesn't work well. I would say that changing themes on a production site is almost completely wrong. Selecting a theme is a staging task, modifying a theme is a staging task. Pushing a theme from version control is where the changes should happen. If that is the case, then preparing widgets should be a staging task as well as on highly trafficked site you don't want to have to race to update your widget configuration after you have changed a theme.
As for the usability issue, that could be solved by showing and loading the widget configurations by theme. Which if you are staging a theme change, you would want to do. Say, I'm thinking about switching to 2011, I would want to prep a widget configuration before making the theme go live, maybe even in functions.php, register the sidebars and the desired widget configuration.
This is all really just broken, widgets are a bad idea because in their current form they encourage onsite edits of something other than content with zero version control. Because widgets are really code modifications. Just because they are wrapped up and have a visual configuration for running the function, doesn't make it any less a modification of which code runs to build your page.
As for the existance of similar functionality in plugins. I agree that this is a core issue. You don't make any progress on common theming standards by leaving versioning of widget configurations in plugins.
Theme developers need a clear expectation of what is going to happen with widgets when someone switches on their theme, if not total control.
The idea of a timeout and progressive mapping also irks me. Consider three sidebars "header", "sidebar", "footer". Then you switch to a theme with "header", "footer". Progressive mapping maps the "sidebar" to the "footer". This is clearly wrong if you have matched names. Then even with a timeout where widget settings magicly get restored, once the timeout is over and you switch back, you now have two sidebars and your "sidebar" is now your "footer".
That seems equally or more dangerous than a persistent configuration. With a persistent configuration you know that you have made the choice of what to put in each theme. And nothing random happens without deliberate user action. So if you switch one or two times you quickly find out that each theme requires a one time config of widgets and that they persist. That is surprising people with control. Automapping is suprising people with a lack of control and deliberately making incorrect assumptions about their intentions. People would never add content to something clearly labeled "footer" and expect that to end up in a sidebar.
Is there really any other way to look at it?
The extra layer of interface function on theme switch should be a notification that you theme has change and you have (n) empty sidebars and (y) sidebars that match by name. Then an ability to copy a config from another theme to your new sidebar. Thought non of that would be critical with persistence.
I would like to see that rolled out on wordpress.com and see how quickly it gets rolled back. :)
#76
@
13 years ago
Trevogre,
For all the trouble you would have to go to with widget configurations via version control, you'd be better off not using widgets at all.
Widgets are designed for non-technical people who don't know PHP, let alone version control. Try to keep that in mind.
#77
@
13 years ago
scribu,
I get that, I think the idea of changing things is to make widgets better. I'm suggesting that get them ready for use with a proper methodology would be a good idea.
Right now a theme developer using version control registers the sidebars with no control over what the suggested widgets are. So they either have to hardcode in basic things like recent posts where they want them and sacrifice configurability by the laymen, or not have any suggested items in their widget areas.
It would seem really benificial to be able to register your sidebars, which you have to do anyway. And be able to define a configuration array to prepopulate the widgets. I'm sure you are right that this is hard, right now, but it could be made less so with changes to core.
Besides which, posts and pages are also for laymen and they have version control baked into the dashboard. I can't think of a good reason why widgets shouldn't also be first class citizens and get versioning somewhere. Other than the effort it would take.
This is especially important for non-techical people because they are they ones that need rollback the most.
What happens now when you give a client admin access because they want it and they think they are smart on go in and change widgets that you configured a year ago. Do you really want to rollback to a backup for that so you can get the settings back, or have to load up a backup on your local machine to snoop the widget configuration on a highly widgetized theme. What a drag.
If widgets are going to stay the way they are now there are more reason to not use them then there are to use them, but its hard to reconcile that with their existance and ease of use. You want to give clients more capability even if it doesn't make sense from a worflow and reliability perspective. And even as a dev, you want to be able to use widgets because they are easy to use.
Is there really a reason why they have to be useful only in the most narrow of circumstances and can't have versioning for thier settings?
As an answer to the versioning issue and clutter. The interface could be just like the post version interface, with may the addition of the theme name as one of the columns. So every time you make changes it versions, and everytime you change themes it versions.
So you wouldn't have the sidebar clutter that some suggested, but you also would never loose your settings.
#78
@
13 years ago
This discussion is way outside the scope of this ticket. I suggest you take a look at #16613 and open a new ticket for widget version control.
#80
in reply to:
↑ 74
@
13 years ago
Replying to aaroncampbell:
Thinking that we should move check_theme_switched()
to wp-includes/theme.php and run an action with it. This will make it usable to plugins and other tasks we may need to do after a theme has been switched, like in #18588.
#83
@
13 years ago
How about renaming the action to 'after_switch_theme'?
It would be consistent with 'switch_theme' and would be a neat fix for #7795.
#84
@
13 years ago
I knew there was a ticket about that somewhere, didn't expect it to be 3 years old...
Sure, we can rename the hook, 'after_switch_theme'
sounds good too.
#88
in reply to:
↑ 87
@
13 years ago
Replying to scribu:
This should not yet be a public API.
Because?
... because there has been no discussion on it.
#90
@
13 years ago
Replying to nacin:
... because there has been no discussion on it.
Hmm, don't see any opinions against having such hook. Perhaps #14849 can be considered fixed with this too http://core.trac.wordpress.org/ticket/14849#comment:9
#91
@
13 years ago
Personally I think the problem with (de)activation hooks isn't that they're bad, but that they're inconsistent in plugins and thus we have a bad taste in our mouths (they're broke so people shouldn't use them).
Anyway, I don't really look at this as an activation hook because it offers something that an activation hook doesn't. It gives plugins the chance to know what the theme WAS as well as what it IS. It's something that seems like it could be useful for any plugins that want to make theme switching better (user-interactive mapping of widgets or menus, maybe migration of certain theme mods, etc).
#93
follow-ups:
↓ 94
↓ 98
@
13 years ago
[18630] causes fatal errors.
retrieve_widgets()
can call wp_get_sidebars_widgets()
, which can call retrieve_widgets()
.
To test, set $sidebars_widgets['array_version']
to 1, for example.
#94
in reply to:
↑ 93
;
follow-ups:
↓ 95
↓ 100
@
13 years ago
Replying to mdawaffe:
To test, set
$sidebars_widgets['array_version']
to 1, for example.
Was thinking it's time to remove that code completely. It was used to convert the $sidebars_widgets
format in WP 2.2 I believe.
Without it the worst case scenario would be that all widgets are dumped in "Inactive", and $sidebars_widgets
will be overwritten on the first movement of a widget on the widgets config screen.
#99
in reply to:
↑ 98
@
13 years ago
- Keywords ux-feedback removed
Replying to SergeyBiryukov:
Replying to mdawaffe:
retrieve_widgets()
can callwp_get_sidebars_widgets()
, which can callretrieve_widgets()
.
The patch on #18598 should fix this.
I refreshed that patch -- that ticket should probably be closed as a duplicate, it's the same issue reported here.
@
13 years ago
Combines patch on #18598 with removing unneeded wp_get_sidebars_widgets() from switch_theme()
#100
in reply to:
↑ 94
;
follow-up:
↓ 102
@
13 years ago
Replying to azaozz:
Replying to mdawaffe:
To test, set
$sidebars_widgets['array_version']
to 1, for example.
Was thinking it's time to remove that code completely. It was used to convert the
$sidebars_widgets
format in WP 2.2 I believe.
Without it the worst case scenario would be that all widgets are dumped in "Inactive", and
$sidebars_widgets
will be overwritten on the first movement of a widget on the widgets config screen.
Why don't we move this code to only run at db upgrade time if needed - that way old old installs that upgrade still get fixed by it doesn't have to run the rest of the time.
#102
in reply to:
↑ 100
@
13 years ago
Replying to westi:
Why don't we move this code to only run at db upgrade time if needed - that way old old installs that upgrade still get fixed by it doesn't have to run the rest of the time.
17979.14.diff is an attempt to do this with a bit of cleanup as well:
- Moves the old
sidebars_widgets
array upgrade code towp-admin/includes/upgrade.php
. - The upgrade code didn't change since 2.8.1, so I compare
$wp_current_db_version
to 11548. - Contains the refreshed patch from #18598 to prevent an infinite loop.
- Removes two unneeded
wp_get_sidebars_widgets()
calls, as suggested by lancewillett. - Removes unused
$wp_registered_sidebars
variable fromwp_get_sidebars_widgets()
. - Combines a couple of
!empty( $sidebars_widgets )
checks inretrieve_widgets()
.
#105
@
13 years ago
I haven't had a chance to do extensive testing, but I like 17979.14.diff. I think it makes for sense for the widget upgrade logic to move, and fewer wp_get_sidebars_widgets() calls is a bonus!
#106
@
13 years ago
- Owner set to ryan
- Resolution set to fixed
- Status changed from new to closed
In [18821]:
#110
follow-up:
↓ 111
@
13 years ago
Jane reported today in IRC #wordpress-dev that this didn't work correctly in testing:
Jane: test site had 2010 as theme, with some widgets. activated 2011 on themes.php. Refreshed itself to themes.php?activated=true and showed alert message " New theme activated. This theme supports widgets, please visit the widgets settings screen to configure them." Go to widgets screen, widgets in primary sidebar in 2010 are not in sidebar, no widgets live anymore
Jane: switch back to 2010, same message appears on themes.php?activated=true and 2010 sidebar comes back
Nacin then chimed in:
nacin: I stumbled across it yesterday and things broke every which way.
I looked through all the files that were modified as part of this functionality change, and I don't see anything that would make the widgets behave differently since we pushed this out two months ago.
I also can't reproduce in testing, using 3.3-beta3-19254. I cleared theme mods out, still works correctly.
We've been running this on WP.com for 5 weeks now, and it's stable.
#111
in reply to:
↑ 110
@
13 years ago
Replying to lancewillett:
I also can't reproduce in testing, using 3.3-beta3-19254. I cleared theme mods out, still works correctly.
I was able to reproduce in one case. See #19291.
#114
@
13 years ago
Far too late to say so, but I really fail to see how this was a bug. Assigning widgets to "Inactive Widgets" when they could not be allocated to an active sidebar by matching IDs? Perfectly logical. Surely people understand that WordPress isn't psychic? No, instead we have some brute force to fit them into active sidebars - even if it means the "Logged In Users Only Sidebar" items get shunted into "Home Page Only Sidebar".
If one theme has a special "404 page sidebar", then why assume it has an equivalent in every theme? If users are expected to move things around, how is "In The Wrong Place" widgets safer than widgets that don't show until they've been taken out from "Inactive Widgets" and put in the appropriate place?
I doubly fail to see why it warranted the creation of a set of auto-expiring backups, and the whole thing trying to second-guess whether you wanted the old widgets or the new widget positions.
Questions:
- If I install a theme and click Preview, does that show me the theme with the current widget arrangement or the arrangement that would be in place if I activated the theme properly?
- If I have "Right Sidebar" in two themes, and a plugin that lets users pick between them, do widget changes in the active theme no longer affect the inactive theme?
- I've lost track. Does this genuinely ignore ID and favour position?
- Does this mean that theme developers should register sidebars in a particular order for maximum compatibility?
#115
@
13 years ago
PaulGregory,
I don't know that is to late, other than then issue of wordpress being fanaticly backwards compatible. Though I'm not sure changing this again could be considered creating an incompatiblity because its really just a functional change to how the ui expresses the configuration and carries them over during theme changes. I would think that this could be updated to be more reliable.
I don't know if you read some of the previous commments but I made a few long ones detailing what I thought was the proper way to move widgets forward. I think at that point there was just to much momentum on the idea of auto allocating them.
I completely agree that auto allocatation is weak. As it doesn't respect sidebar context at all.
I did think it was a bug because when the items dropped to inactive widgets they would lose their group on sidebars. So in an environment with multiple admins or just if you weren't paying close attention it would be really easy to not recall the proper configuration after your widgets got dumped.
To my knowledge this patch didn't solve that problem in any meaningfull way.
In my mind there is a much larger issue here. That changing anything to a match a theme that is outside the theme is a recipe for losing your settings. I would rather have every wordpress setting have unique data for each theme that is enabled. Widgets, enabled plugins, and anything that might need to change to suit a different theme.
I love the idea of being able to change themes, but currently there really is no point. You have to pick your theme when you begin development and never change it unless you want to really confuse yourself and risk blowing up your site.
Unless you use totally benign themes with almost zero configuration or customization.
Anyway, +1 to your commentary. I would like to see this revisited.
#117
follow-up:
↓ 124
@
13 years ago
Long time reader, first time commenter. I would have joined in earlier, but ideas were already set when I saw this ticket.
There are a couple of things that I haven't seen people directly address in this discussion:
- Themes aren't the only source of sidebars. The following are a few plugins that I found in about ten minutes of searching: Core Sidebars, Custom sidebars, Dynamics Sidebars, Sidebar Generator, Tabbed Widgets, Tabber Widget, Widgets on Pages, and WP Sidebars. Not only do these plugins add sidebars, most of them offer a variable number of sidebars, so the addition of new sidebars and the removal of current ones are relatively-common occurrences that don't involve changes in the theme.
- There are a number of themes that support variable sidebars either though options that add and remove sidebars or through filters that allow for control over the sidebars. Some examples that I know of: Builder by iThemes, Canvas by WooThemes, Catalyst, EvoLve by Theme4Press, Genesis by StudioPress, Headway, Hybrid by Theme Hybrid, StartBox, Thematic, and WP Paintbrush. As with the plugins listed above, sidebars appearing and disappearing during use are not rare events.
Given the current state of this solution and the information given above, seeing widgets pop into Inactive Sidebar locations is not limited to just theme changes. Personally, I'm not a huge fan of this, but I do find it acceptable. The main thing I'm concerned about is how the text for the Inactive Sidebars very explicitly limits the explanation to an old theme / new theme relationship. Since it is quite possible for these Inactive Sidebars to be created via other actions, this could be confusing to users.
All the browsers I have tested (Firefox and Chrome in Ubuntu and IE 9 in Win 7) cut off the Inactive Sidebar titles to "Inactive Sidebar (from previous them" due to it being too long to fit in the box, so shortening is needed. In addition, there is no explanation to users on how to remove the Inactive Sidebar from the Widgets page. Since this is considered to be a fairly rare occurrence for most users, I think having details in the description could be helpful.
I'm attaching a patch with a proposed change of text. I'm not attached to the wording, but I do want to ensure that the wording doesn't limit the explanation to just a theme switch.
Current trunk Inactive Sidebar title and description:
Inactive Sidebar (from previous theme)
This is a left over sidebar from an old theme and does not show anywhere on your site
Title and description supplied in patch:
Inactive Sidebar (not used)
This sidebar that is no longer available and does not show anywhere on your site. Remove each of the widgets below to fully remove this inactive sidebar.
I should note that I am very glad that people settled on the current solution (Inactive Sidebar listings) rather than the forced filling of sidebars. This is because I saw widget shuffling while using some of the above-mentioned plugins and themes due to the attempt to migrate orphaned widgets to existing sidebars. This was definitely not desirable.
#118
follow-up:
↓ 119
@
13 years ago
The "(from previous theme)" is still cut off and remains unfixed. Re-opening. +1 on chris's patch, at a glance. The grammar needs work though.
#119
in reply to:
↑ 118
@
13 years ago
Replying to nacin:
The "(from previous theme)" is still cut off and remains unfixed. Re-opening. +1 on chris's patch, at a glance. The grammar needs work though.
Eww... How did that get there. Updated the patch with the more reasonable "This sidebar is no longer..."
#120
@
13 years ago
Just jumping in, and apologies if this has been mentioned in the previous 119 (!!!) comments on this ticket, but possibly useful in the long run working of things...
What if sidebars were stored as-is on theme change, but inactive if it doesn't match a sidebar id in the new theme with an option to re-assign it to one of the sidebars in the new theme?
For example Theme A had a sidebar called right-sidebar and Theme B had a sidebar called Sidebar1. When switching the previous sidebar (right-sidebar) would be stored in its previous state, shown as inactive, and given the option to assign it to one of the sidebar ids in the current theme. This obviously would be more useful in a theme with many sidebars for pages, single, homepage, etc... its a lot easier to re-assign them to the correct sidebar areas in the new theme.
This may need to be something hashed out later in the spirit of getting 3.3 wrapped up and out the door and thats totally fine, just voicing my idea as one way it might work.
#122
@
13 years ago
- Cc meglio added
Hello WP Hackers!
Sorry for this off-topic question but I'm sure it is the most right place to ask it.
So I'm programmatically switching theme with switch_theme(), then wanting to add/sort widgets in the newly registered sidebar, but before this I want WP to process orphaned widgets and put them in the sidebar of the new theme. Is there any way to do it programmatically - some function to call after switch_theme()? Thanks!
#123
@
13 years ago
This is a closed ticket, and is also not a support venue. You might try the forums, the wp-hackers list, or perhaps even WPSE.
#124
in reply to:
↑ 117
@
13 years ago
I completely agree that the Inactive Sidebar listings is the best solution, but i which there was a better (more solid/robust) solution for handeling the mappign in new themes and especially when no sidebar is in the theme.
/Event.
Replying to chrisbliss18:
Long time reader, first time commenter. I would have joined in earlier, but ideas were already set when I saw this ticket.
There are a couple of things that I haven't seen people directly address in this discussion:
- Themes aren't the only source of sidebars. The following are a few plugins that I found in about ten minutes of searching: Core Sidebars, Custom sidebars, Dynamics Sidebars, Sidebar Generator, Tabbed Widgets, Tabber Widget, Widgets on Pages, and WP Sidebars. Not only do these plugins add sidebars, most of them offer a variable number of sidebars, so the addition of new sidebars and the removal of current ones are relatively-common occurrences that don't involve changes in the theme.
- There are a number of themes that support variable sidebars either though options that add and remove sidebars or through filters that allow for control over the sidebars. Some examples that I know of: Builder by iThemes, Canvas by WooThemes, Catalyst, EvoLve by Theme4Press, Genesis by StudioPress, Headway, Hybrid by Theme Hybrid, StartBox, Thematic, and WP Paintbrush. As with the plugins listed above, sidebars appearing and disappearing during use are not rare events.
Given the current state of this solution and the information given above, seeing widgets pop into event Sidebar locations is not limited to just theme changes. Personally, I'm not a huge fan of this, but I do find it acceptable. The main thing I'm concerned about is how the text for the Inactive Sidebars very explicitly limits the explanation to an old theme / new theme relationship. Since it is quite possible for these Inactive Sidebars to be created via other actions, this could be confusing to users.
All the browsers I have tested (Firefox and Chrome in Ubuntu and IE 9 in Win 7) cut off the aarhus Sidebar titles to "Inactive Sidebar (from previous them" due to it being too long to fit in the box, so shortening is needed. In addition, there is no explanation to users on how to remove the Inactive Sidebar from the Widgets page. Since this is considered to be a fairly rare occurrence for most users, I think having details in the description could be helpful.
I'm attaching a patch with a proposed change of text. I'm not attached to the wording, but I do want to ensure that the wording doesn't limit the explanation to just a theme switch.
Current trunk Inactive Sidebar title and description:
Inactive Sidebar (from previous theme)
This is a left over sidebar from an old theme and does not show anywhere on your site
Title and description supplied in patch:
Inactive Sidebar (not used)
This sidebar that is no longer available and does not show anywhere on your site. Remove each of the widgets below to fully remove this inactive sidebar.
I should note that I am very glad that people settled on the current solution (Inactive Sidebar listings) rather than the forced filling of sidebars. This is because I saw widget shuffling while using some of the above-mentioned plugins and themes due to the attempt to migrate orphaned widgets to existing sidebars. This was definitely not desirable.
Here's my suggested solution:
Add a third step to
retrieve_widgets()
to move all to first registered sidebar after "good ones" and "same number of sidebars" checks.switch_themes()
should call the widget switch function explicitly, instead of relying on it being called fromdynamic_sidebar
or other functions.This looks like it will need to happen in
retrieve_widgets()
inwp-admin/widgets.php
(starting around line 66).Then in
wp_get_sidebars_widgets()
improve the case 2 to follow this same logic. (Seems like switch_theme() should run the retrieve_widgets function and there shouldn't be the same code in two places.)