-
Notifications
You must be signed in to change notification settings - Fork 13
fix(newspack-plugin): keep admin list-table Title from collapsing #595
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
Draft
jason10lee
wants to merge
6
commits into
main
Choose a base branch
from
fix/nppm-2989-admin-list-table-autolayout
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
c9c4279
fix(admin): add list-table layout screen registry + base-gated matching
jason10lee 0306eae
fix(admin): emit table-layout:auto + validated primary min-width
jason10lee e30152a
fix(admin): load and hook the list-table auto-layout helper
jason10lee 95d5abc
style(admin): test doc comments + docblock/regex/test-state polish
jason10lee 861c981
fix(admin): reject zero min-width, drop stale doc ref, isolate tests
jason10lee 607fc39
Merge branch 'main' into fix/nppm-2989-admin-list-table-autolayout
jason10lee File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
160 changes: 160 additions & 0 deletions
160
plugins/newspack-plugin/includes/class-admin-list-table-layout.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,160 @@ | ||
| <?php | ||
| /** | ||
| * Admin list-table layout legibility. | ||
| * | ||
| * @package Newspack | ||
| */ | ||
|
|
||
| namespace Newspack; | ||
|
|
||
| defined( 'ABSPATH' ) || exit; | ||
|
|
||
| /** | ||
| * Keeps Newspack's admin list-table screens legible when many plugins add | ||
| * columns and over-subscribe the fixed-layout table (collapsing Title to 0). | ||
| * Switches those screens to `table-layout: auto` and floors the primary column. | ||
| */ | ||
| class Admin_List_Table_Layout { | ||
|
|
||
| /** | ||
| * Screens treated by default. Each entry is a post_type, taxonomy, or screen id. | ||
| * Overridable (add or remove) via the `newspack_admin_autolayout_screens` filter. | ||
| */ | ||
| const DEFAULT_SCREENS = [ 'post', 'page' ]; | ||
|
|
||
| /** | ||
| * Default primary-column min-width floor. `35ch` ≈ 300px at the default admin | ||
| * font and scales with it. Overridable via `newspack_admin_primary_column_min_width`. | ||
| */ | ||
| const DEFAULT_MIN_WIDTH = '35ch'; | ||
|
|
||
| /** | ||
| * Allowed min-width syntax: a positive, non-zero integer plus a length unit. | ||
| * Zero is excluded so a `0px`/`0ch` filter value can't defeat the floor; | ||
| * percentage is excluded (ignored on table cells); the `D` flag anchors the | ||
| * end so a trailing newline is rejected too. | ||
| */ | ||
| const MIN_WIDTH_PATTERN = '/^[1-9]\d*(?:px|ch|rem)$/D'; | ||
|
|
||
| /** | ||
| * Extra screens registered on top of DEFAULT_SCREENS. | ||
| * | ||
| * @var string[] | ||
| */ | ||
| private static $registered = []; | ||
|
|
||
| /** | ||
| * Hook style output onto list-table screens. | ||
| */ | ||
| public static function init() { | ||
| add_action( 'admin_head-edit.php', [ __CLASS__, 'render_styles' ] ); | ||
| add_action( 'admin_head-edit-tags.php', [ __CLASS__, 'render_styles' ] ); | ||
| } | ||
|
|
||
| /** | ||
| * Register an additional screen for the auto-layout treatment. | ||
| * | ||
| * @param string $key A post_type, taxonomy, or screen id. | ||
| */ | ||
| public static function register_screen( $key ) { | ||
| if ( is_string( $key ) && '' !== $key ) { | ||
| self::$registered[] = $key; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * The full set of treated screen keys: defaults + registered, then filtered. | ||
| * | ||
| * @return string[] | ||
| */ | ||
| public static function get_screens() { | ||
| $screens = array_merge( self::DEFAULT_SCREENS, self::$registered ); | ||
|
|
||
| /** | ||
| * Filters the admin list-table screens that receive the auto-layout | ||
| * treatment. May add or remove entries, including the `post`/`page` | ||
| * defaults. | ||
| * | ||
| * @param string[] $screens Screen keys (post_type, taxonomy, or screen id). | ||
| */ | ||
| $screens = apply_filters( 'newspack_admin_autolayout_screens', array_values( array_unique( $screens ) ) ); | ||
|
|
||
| return array_values( array_filter( (array) $screens ) ); | ||
| } | ||
|
|
||
| /** | ||
| * Whether a screen is treated. Base-gated: a `post` key matches the Posts | ||
| * list (`edit.php`, base `edit`) but never the Categories/Tags term screens | ||
| * (`edit-tags.php`, base `edit-tags`) which also carry `post_type=post`. | ||
| * | ||
| * @param \WP_Screen $screen Screen to test. | ||
| * @return bool | ||
| */ | ||
| public static function screen_matches( $screen ) { | ||
| if ( ! $screen instanceof \WP_Screen ) { | ||
| return false; | ||
| } | ||
| $keys = self::get_screens(); | ||
| if ( empty( $keys ) ) { | ||
| return false; | ||
| } | ||
| $candidates = [ isset( $screen->id ) ? $screen->id : '' ]; | ||
| if ( 'edit' === $screen->base ) { | ||
| $candidates[] = isset( $screen->post_type ) ? $screen->post_type : ''; | ||
| } elseif ( 'edit-tags' === $screen->base ) { | ||
| $candidates[] = isset( $screen->taxonomy ) ? $screen->taxonomy : ''; | ||
| } | ||
| $candidates = array_filter( $candidates ); | ||
| return (bool) array_intersect( $keys, $candidates ); | ||
| } | ||
|
|
||
| /** | ||
| * Resolve the validated primary-column min-width floor. | ||
| * | ||
| * @return string A valid CSS length (px|ch|rem); the default on invalid input. | ||
| */ | ||
| public static function get_min_width() { | ||
| /** | ||
| * Filters the primary (Title) column min-width floor on treated screens. | ||
| * | ||
| * @param string $min_width A CSS length: px, ch, or rem (default '35ch'). | ||
| */ | ||
| $min_width = apply_filters( 'newspack_admin_primary_column_min_width', self::DEFAULT_MIN_WIDTH ); | ||
| return preg_match( self::MIN_WIDTH_PATTERN, (string) $min_width ) ? (string) $min_width : self::DEFAULT_MIN_WIDTH; | ||
| } | ||
|
|
||
| /** | ||
| * Build the CSS for a screen. Empty string unless the screen is treated. | ||
| * | ||
| * @param \WP_Screen $screen Screen to build CSS for. | ||
| * @return string CSS block (no <style> wrapper); '' when not treated. | ||
| */ | ||
| public static function get_styles_for_screen( $screen ) { | ||
| if ( ! self::screen_matches( $screen ) ) { | ||
| return ''; | ||
| } | ||
| $min_width = self::get_min_width(); | ||
| return "@media screen and (min-width: 783px) {\n" | ||
| . "\t.wp-list-table.fixed { table-layout: auto; }\n" | ||
| . "\t.wp-list-table th.column-primary,\n" | ||
| . "\t.wp-list-table td.column-primary { min-width: " . $min_width . "; }\n" | ||
| . '}'; | ||
| } | ||
|
|
||
| /** | ||
| * Echo the scoped <style> block for the current screen. | ||
| */ | ||
| public static function render_styles() { | ||
| $screen = get_current_screen(); | ||
| if ( ! $screen ) { | ||
| return; | ||
| } | ||
| $styles = self::get_styles_for_screen( $screen ); | ||
| if ( '' === $styles ) { | ||
| return; | ||
| } | ||
| // Fixed literal CSS plus an allowlist-validated length (get_min_width) — safe. | ||
| echo "<style id=\"newspack-admin-list-table-layout\">\n" . $styles . "\n</style>\n"; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped | ||
| } | ||
| } | ||
| Admin_List_Table_Layout::init(); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
178 changes: 178 additions & 0 deletions
178
plugins/newspack-plugin/tests/unit-tests/test-admin-list-table-layout.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,178 @@ | ||
| <?php | ||
| /** | ||
| * Tests for admin list-table auto-layout. | ||
| * | ||
| * @package Newspack\Tests | ||
| */ | ||
|
|
||
| namespace Newspack\Tests; | ||
|
|
||
| use Newspack\Admin_List_Table_Layout; | ||
|
|
||
| /** | ||
| * Tests for the Admin_List_Table_Layout helper. | ||
| * | ||
| * @group admin-list-table-layout | ||
| */ | ||
| class Test_Admin_List_Table_Layout extends \WP_UnitTestCase { | ||
|
|
||
| /** | ||
| * Load the class under test once for the whole test case. | ||
| */ | ||
| public static function set_up_before_class(): void { | ||
| parent::set_up_before_class(); | ||
| require_once NEWSPACK_ABSPATH . 'includes/class-admin-list-table-layout.php'; | ||
| } | ||
|
|
||
| /** | ||
| * Reset shared state so filters and register_screen() calls can't bleed across tests. | ||
| */ | ||
| public function tear_down(): void { | ||
| remove_all_filters( 'newspack_admin_autolayout_screens' ); | ||
| remove_all_filters( 'newspack_admin_primary_column_min_width' ); | ||
| $registered = new \ReflectionProperty( Admin_List_Table_Layout::class, 'registered' ); | ||
| $registered->setAccessible( true ); | ||
| $registered->setValue( null, [] ); | ||
| parent::tear_down(); | ||
| } | ||
|
|
||
| /** | ||
| * Pin the screen set so tests are independent of register_screen() bleed. | ||
| * | ||
| * @param string[] $screens Screen keys to force as the treated set. | ||
| */ | ||
| private function pin_screens( array $screens ): void { | ||
| add_filter( 'newspack_admin_autolayout_screens', fn() => $screens ); | ||
| } | ||
|
|
||
| /** | ||
| * The default treated set includes the post and page list screens. | ||
| */ | ||
| public function test_defaults_include_post_and_page(): void { | ||
| $screens = Admin_List_Table_Layout::get_screens(); | ||
| $this->assertContains( 'post', $screens ); | ||
| $this->assertContains( 'page', $screens ); | ||
| } | ||
|
|
||
| /** | ||
| * Registering a screen adds its key to the treated set. | ||
| */ | ||
| public function test_register_screen_adds_key(): void { | ||
| Admin_List_Table_Layout::register_screen( 'my_cpt' ); | ||
| $this->assertContains( 'my_cpt', Admin_List_Table_Layout::get_screens() ); | ||
| } | ||
|
|
||
| /** | ||
| * The Posts list screen (edit.php, base edit) matches the 'post' key. | ||
| */ | ||
| public function test_matches_post_list_screen(): void { | ||
| $this->pin_screens( [ 'post', 'page' ] ); | ||
| $this->assertTrue( Admin_List_Table_Layout::screen_matches( \WP_Screen::get( 'edit-post' ) ) ); | ||
| } | ||
|
|
||
| /** | ||
| * The Categories term screen must not match despite carrying post_type=post. | ||
| */ | ||
| public function test_leak_guard_category_term_screen_does_not_match(): void { | ||
| $this->pin_screens( [ 'post', 'page' ] ); | ||
| // edit-category carries post_type=post but base=edit-tags — must NOT match. | ||
| $this->assertFalse( Admin_List_Table_Layout::screen_matches( \WP_Screen::get( 'edit-category' ) ) ); | ||
| } | ||
|
|
||
| /** | ||
| * The Tags term screen must not match despite carrying post_type=post. | ||
| */ | ||
| public function test_leak_guard_tag_term_screen_does_not_match(): void { | ||
| $this->pin_screens( [ 'post', 'page' ] ); | ||
| $this->assertFalse( Admin_List_Table_Layout::screen_matches( \WP_Screen::get( 'edit-post_tag' ) ) ); | ||
| } | ||
|
|
||
| /** | ||
| * A post type that is not in the treated set does not match. | ||
| */ | ||
| public function test_unregistered_post_type_does_not_match(): void { | ||
| register_post_type( 'np_test_cpt', [ 'public' => true ] ); | ||
| $this->pin_screens( [ 'post', 'page' ] ); | ||
| $this->assertFalse( Admin_List_Table_Layout::screen_matches( \WP_Screen::get( 'edit-np_test_cpt' ) ) ); | ||
| unregister_post_type( 'np_test_cpt' ); | ||
| } | ||
|
|
||
| /** | ||
| * The filter can remove a default screen from the treated set. | ||
| */ | ||
| public function test_filter_can_remove_a_default(): void { | ||
| $this->pin_screens( [ 'page' ] ); // 'post' removed. | ||
| $this->assertFalse( Admin_List_Table_Layout::screen_matches( \WP_Screen::get( 'edit-post' ) ) ); | ||
| } | ||
|
|
||
| /** | ||
| * Untreated screens emit no CSS. | ||
| */ | ||
| public function test_no_emission_for_untreated_screen(): void { | ||
| register_post_type( 'np_test_cpt', [ 'public' => true ] ); | ||
| $this->pin_screens( [ 'post', 'page' ] ); | ||
| $this->assertSame( '', Admin_List_Table_Layout::get_styles_for_screen( \WP_Screen::get( 'edit-np_test_cpt' ) ) ); | ||
| unregister_post_type( 'np_test_cpt' ); | ||
| } | ||
|
|
||
| /** | ||
| * Term screens emit no CSS (leak guard at the emission layer). | ||
| */ | ||
| public function test_leak_guard_emits_nothing_on_term_screen(): void { | ||
| $this->pin_screens( [ 'post', 'page' ] ); | ||
| $this->assertSame( '', Admin_List_Table_Layout::get_styles_for_screen( \WP_Screen::get( 'edit-category' ) ) ); | ||
| } | ||
|
|
||
| /** | ||
| * A treated screen emits the desktop-scoped auto-layout CSS with the floor. | ||
| */ | ||
| public function test_emits_auto_layout_for_post_list(): void { | ||
| $this->pin_screens( [ 'post', 'page' ] ); | ||
| $css = Admin_List_Table_Layout::get_styles_for_screen( \WP_Screen::get( 'edit-post' ) ); | ||
| $this->assertStringContainsString( '@media screen and (min-width: 783px)', $css ); | ||
| $this->assertStringContainsString( '.wp-list-table.fixed { table-layout: auto; }', $css ); | ||
| $this->assertStringContainsString( 'column-primary', $css ); | ||
| $this->assertStringContainsString( 'min-width: 35ch;', $css ); | ||
| } | ||
|
|
||
| /** | ||
| * The min-width floor defaults to 35ch. | ||
| */ | ||
| public function test_min_width_default(): void { | ||
| $this->assertSame( '35ch', Admin_List_Table_Layout::get_min_width() ); | ||
| } | ||
|
|
||
| /** | ||
| * The min-width filter accepts px, ch, and rem length values. | ||
| */ | ||
| public function test_min_width_accepts_px_and_ch_and_rem(): void { | ||
| foreach ( [ '280px', '40ch', '20rem' ] as $value ) { | ||
| add_filter( 'newspack_admin_primary_column_min_width', fn() => $value ); | ||
| $this->assertSame( $value, Admin_List_Table_Layout::get_min_width() ); | ||
| remove_all_filters( 'newspack_admin_primary_column_min_width' ); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * The min-width filter rejects junk, percentages, zero, and trailing newlines. | ||
| */ | ||
| public function test_min_width_rejects_junk_and_percentage(): void { | ||
| foreach ( [ '30%', '100', 'red;}', 'expression(alert(1))', '35 ch', "35ch\n", '0px', '0ch' ] as $junk ) { | ||
| add_filter( 'newspack_admin_primary_column_min_width', fn() => $junk ); | ||
| $this->assertSame( '35ch', Admin_List_Table_Layout::get_min_width(), "rejected: $junk" ); | ||
| remove_all_filters( 'newspack_admin_primary_column_min_width' ); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * The init() self-call registers the admin_head hooks on require. | ||
| */ | ||
| public function test_init_registers_admin_head_hooks(): void { | ||
| $this->assertNotFalse( | ||
| has_action( 'admin_head-edit.php', [ Admin_List_Table_Layout::class, 'render_styles' ] ) | ||
| ); | ||
| $this->assertNotFalse( | ||
| has_action( 'admin_head-edit-tags.php', [ Admin_List_Table_Layout::class, 'render_styles' ] ) | ||
| ); | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.