Undefined variable $screen_layout_columns when filtering out screen options

[griffinjt]

When using the screen_options_show_screen filter to remove the screen tab completely, an undefined variable notice appears with WP_DEBUG on. Specifically, the $screen_layout_columns variable is undefined because the current code expects it to be set and to be an integer.

This patch fixes it by setting it equal to itself while still casting it as an integer. Can't find any issues in my testing.

[SergeyBiryukov]

There are some more instances of using unchecked $screen_layout_columns value (20506.patch​). Perhaps we should set it somewhere outside of WP_Screen::render_screen_layout() if the latter is not called.

[griffinjt]

Yes, it needs to be outside out that method.

Actually, the whole method needs to be separated into two methods: one with the logic and the other with the output. I've split WP_Screen::render_screen_layout() into the former and WP_Screen::get_screen_layout().

Since it's separated, I've called WP_Screen::get_screen_layout() within the render_screen_meta method, just before the check is made to see if the screen options should be output.

Needs some testing, but it has worked fine for me in all my tests.

[ryan]

nacin and I discussed introducing get_user_option() and deprecating the global.

Lines like this:

echo (int) $screen_layout_columns ? (int) $screen_layout_columns : 'auto';

Would become:

echo get_current_screen()->get_user_option( 'layout_columns', 'auto' );

[nacin]

echo get_current_screen()->get_user_option( 'layout_columns', 'auto' );

The one issue with that is the second argument is $default in this case, which is not unusual. (get_option() works the same way.) But it would be inconsistent with the WP_Screen::get_option() method, whose second argument is a specific array key of the requested option, rather than a default. I'm now kicking myself for that.

[nacin]

I also just don't know how many of these $screen_layout_columns calls are necessary for functionality. Some of them were added in [18736] but most of that got backed out of core.

[azaozz]

Replying to nacin:

$screen_layout_columns was mostly needed when we used to output the right postbox container on the Edit/Write screens dependent on how many columns were selected. As that changed in 3.4 and the placement of that postbox container is fully controlled from css, perhaps we can retire the global completely.

[azaozz]

Replying to azaozz:

Hmm, or maybe not, I take this back :)

Implementing the user setting for columns is much more straightforward when having a method (or a global).

[SergeyBiryukov]

Related: [20570]

[griffinjt]

Replying to azaozz:

Replying to azaozz:

Hmm, or maybe not, I take this back :)

Implementing the user setting for columns is much more straightforward when having a method (or a global).

In light of [20570], I don't see why would couldn't retire the global now. :-)

[ryan]

The screen_layout_columns global is still used in edit-form-advanced.php, edit-link-form.php, and dashboard.php. The remaining uses are internal to WP_Screen.

[azaozz]

Right, [20570] was just a cleanup. The way columns work is by setting a class on the container depending on the user setting, then if the screen is narrow, overwriting the CSS to move the column(s) to the right.

[griffinjt]

Ahh, gotcha. Thanks for explaining. :-)

The global could be converted into a static method I suppose, although the global appears to work fine.

[ryan]

Store layout columns in num_columns option rather than a global. Deprecate the global. Use get_option( 'num_columns' ) instead of the global.

[ryan]

Move layout column setup into render_screen_meta(). Use add_option( 'num_columns' ) and get_option( 'num_columns' ) for setting and getting the user selected number of columns. Retain the screen_layout_columns global for back compat. Switch admin screens from the global to get_option().

[ryan]

Cast user option to int. Remove auto check.

[ryan]

get_columns()

[ryan]

Store num columns in object var.

[ryan]

Tidying

[ryan]

In [20579]:

Clean out layout columns API in WP_Screen.

• Move layout column setup into render_screen_meta() so that the number of columns is available earlier.
• Store the user provisioned number of columns in an instance var.
• Access the var with get_columns()
• Move all templates away from the screen_layout_columns global to the get_columns() method.
• Deprecate the global
• Remove the no longer needed check for 'auto' in the user option.
• Cast the user option to an int.

Props griffinjt
fixes #20506

[ryan]

griffinjt, since my patch is a bit different, please try it out and let us know if it solves the problem you're seeing.

[griffinjt]

Replying to ryan:

Looking good on my end - no issues that I can see. The only thing I would change in your patch (nitpicky) is that the way you have called the global doesn't follow coding standards - see ​https://twitter.com/#!/nacin/status/192611263321358338 by nacin regarding the issue.

Should be $GLOBALS['screen_layout_columns'] instead (only spaces is the key is a variable).

[SergeyBiryukov]

There's a typo in the comment (20506.minor-typo-fix.patch​).

[azaozz]

In [20591]:

Fix typo in a comment, props SergeyBiryukov, fixes #20506