Skip to content

fix: UI review bugs - #414

Closed
imantsk wants to merge 67 commits into
core-betafrom
fix/166-ui-review-fixes
Closed

fix: UI review bugs#414
imantsk wants to merge 67 commits into
core-betafrom
fix/166-ui-review-fixes

Conversation

@imantsk

@imantsk imantsk commented Jul 13, 2026

Copy link
Copy Markdown
Member

Integration branch for the admin UI review work. Includes #415 and #422.

Changes

Manage menu structure

  • Manage menu responsibilities split into Manage_Menu_Assets, Manage_Menu_Bulk_Download, Manage_Menu_Screen_Options and Snippet_Type_Counter.
  • Shared ListTable, table navigation and pagination components used by both the snippets table and the cloud tables.
  • Snippets and community cloud results render in either table or card view; table remains the default.
  • Shared SCSS partials for theme tokens, badges, checkboxes, kebab menus, modals, subnav, toolbar and page header.

Snippet preview modal

  • Read-only editor that honours the configured editor theme, with line numbers and a language badge.
  • Minimum and maximum dimensions with internal scrolling.
  • Falls back to a plain code view when the code editor is unavailable, and loads only the assets that view needs.
  • Modal implementation compatible with the minimum supported WordPress version.

Community cloud

  • Snippet descriptions are sanitised in Cloud_Snippet, covering every decode path, and rendered as plain text in the card and table views.
  • Page numbers returned by the API are converted for display; featured results use the cloud search page size.
  • REST collection arguments registered with named keys.
  • Row selection added to the cloud table view, and the tab count restored.

Admin notices

  • Admin\Notice_Filter removes notices that do not originate from the plugin on plugin screens, with a CSS fallback. Gated by the code_snippets/admin/filter_foreign_notices filter, enabled by default.
  • Plugin notices carry the code-snippets-notice class so version switch, migration and React notices stay visible.

Accessibility and styling

  • Locked badge contrast raised to meet WCAG AA.
  • Snippet type badges receive a focus ring in both list and card views.
  • Plugin checkbox styling applied consistently, including Screen Options.
  • Kebab menu row heights and padding corrected; row action hover colour matched to snippet name links.

Maintenance

  • stripTags simplified, with linear handling of comments, malformed tags and block separation.
  • Snippet type counts read through get_snippets() instead of a direct database query.
  • Model::set_fields() ignores input that is neither an array nor an object.
  • Direct access guards removed from autoloaded menu classes.
  • Translator comments used in place of context strings for the cloud author and view count labels.

Testing

  • New PHPUnit coverage for the notice filter, manage menu services, snippet type counter, cloud snippet decoding and preferences REST controller.
  • New Playwright specs for the preview modal, notice filtering, badge contrast, cloud featured results and text utilities.
  • CI passes: PHPUnit on PHP 7.4 through 8.4, both Playwright projects, and lint:js, lint:styles and lint:php.

@imantsk
imantsk force-pushed the fix/166-ui-review-fixes branch from 871c3da to d3b8440 Compare July 14, 2026 10:15
@imantsk imantsk changed the title fix: UI review bugs (#166) — core fix: UI review bugs (#166) Jul 14, 2026
@imantsk
imantsk marked this pull request as ready for review July 14, 2026 12:52
@code-snippets-bot

code-snippets-bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Two actionable issues remain in this PR's exact diff:

  • High — TypeScript regression in the preview modal. src/js/components/common/SnippetPreviewModal.tsx:61-66 passes a JSX <span> to the WordPress Modal title prop, which is declared as title?: string. tsc --noEmit reports TS2322: Type 'Element' is not assignable to type 'string'. Keep title as the snippet-name string and render the badge separately, or provide an accessible custom header without weakening the type.
  • Medium — Avoid loading the full editable-editor stack for a read-only preview. src/php/Admin/Menus/Manage_Menu.php:222 calls enqueue_code_editor('php'), which also enqueues HTMLHint, CSSLint, JSHint, and the plugin's editable-editor bundle. These load on every Manage screen even when the preview never opens. Enqueue only the core CodeMirror assets/settings and selected theme required by the read-only modal.

@imantsk

imantsk commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

Fixed the modal title contract and limited the manage screen to preview-only CodeMirror assets. Verified with lint, PHPUnit, build, and targeted E2E checks.

@code-snippets-bot

Copy link
Copy Markdown
Contributor

One preview regression remains for users who disable syntax highlighting. enqueue_code_preview_editor() relies on wp_enqueue_code_editor() (src/php/Utils/editor.php:115-123), but WordPress returns false and does not enqueue code-editor when the current user's syntax_highlighting preference is disabled. SnippetPreviewModal.tsx:43 then unconditionally calls window.wp.codeEditor.initialize(...), so opening Preview throws instead of displaying the code. Please guard initialization and retain the read-only textarea as a functional fallback when wp.codeEditor is unavailable, and add coverage for the disabled-preference path.

@imantsk

imantsk commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

Guarded the editor bootstrap so the read-only textarea remains a functional fallback when wp.codeEditor is unavailable, and added E2E coverage for the disabled syntax-highlighting preference.

@code-snippets-bot

Copy link
Copy Markdown
Contributor

Follow-up adversarial re-audit found two remaining issues:

  1. The fallback E2E test depends on a view it does not establish. tests/e2e/code-snippets-preview.spec.ts:43-49 uses the helper and selector for .wp-list-table, but this PR still defaults an unset snippet-view preference to card in Preferences_REST_Controller.php:42. On a clean test user, the test can time out before opening Preview; Playwright is also skipped in the current checks. Please explicitly set the preference to table in setup, or exercise the card UI, so the fallback path is actually covered.

  2. The notice ownership check accepts similarly prefixed sibling directories. Notice_Filter.php:155 uses 0 === strpos( $file, dirname( PLUGIN_FILE ) ). A callback file under a sibling such as code-snippets-extra/ therefore passes as plugin-owned and its notice survives filtering. Please compare normalized paths with a trailing directory-separator boundary and add a sibling-prefix regression case.

@imantsk

imantsk commented Jul 18, 2026

Copy link
Copy Markdown
Member Author

Resolved both follow-ups. The fallback E2E setup now explicitly establishes table view before using table selectors, and notice ownership compares normalized plugin-root paths with a directory-separator boundary, including a sibling-prefix regression case. The relevant tests, lint, and build pass.

@code-snippets-bot

Copy link
Copy Markdown
Contributor

Round 4 found two issues in this PR's own head:

  1. Notice_Filter::is_code_snippets_screen() checks only each menu's primary hookname (src/php/Admin/Notice_Filter.php:114-121). The editor's separate Add New hook is therefore not recognized, so foreign notices remain visible on that Code Snippets screen. Please include every hook registered by a menu and add behavioral coverage for Add New. The later foundation branch already contains this repair, so the smallest fix is to bring that implementation and test back here.

  2. SnippetPreviewModal.tsx uses CodeMirror's readOnly: 'nocursor'. In CodeMirror 5 that mode disallows focusing the editor, which prevents keyboard users from reaching and selecting the preview text. Use readOnly: true instead; the later preview-actions branch already contains the canonical fix and keyboard coverage.

@code-snippets-bot code-snippets-bot added the build Adding this label will trigger the zip build action label Jul 23, 2026
@code-snippets-bot code-snippets-bot added the run-tests Trigger automated tests label Jul 23, 2026
@imantsk imantsk removed the build Adding this label will trigger the zip build action label Jul 28, 2026
Comment thread src/js/components/common/ListTable/TablePaginationNavigation.tsx
Comment thread src/js/components/common/ListTable/TablePaginationNavigation.tsx
Comment thread src/js/components/common/ListTable/TablePaginationNavigation.tsx
Comment thread src/js/components/common/ListTable/TablePaginationNavigation.tsx
Comment thread src/js/components/common/ListTable/TablePaginationNavigation.tsx
@imantsk imantsk added the build Adding this label will trigger the zip build action label Jul 28, 2026
@code-snippets-bot

code-snippets-bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Download and install

📦 code-snippets.4.0.0-beta.2.zip

@sheabunge sheabunge closed this Jul 29, 2026
@sheabunge
sheabunge deleted the fix/166-ui-review-fixes branch July 29, 2026 05:19
@sheabunge sheabunge mentioned this pull request Jul 29, 2026
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Adding this label will trigger the zip build action run-tests Trigger automated tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants