Skip to content

fix: beta issues#337

Open
imantsk wants to merge 80 commits intocore-betafrom
fix/beta-issues
Open

fix: beta issues#337
imantsk wants to merge 80 commits intocore-betafrom
fix/beta-issues

Conversation

@imantsk
Copy link
Member

@imantsk imantsk commented Mar 10, 2026

Summary

This branch fixes the beta issues reported for Code Snippets and includes a set of follow-up admin usability improvements that were discovered while validating those fixes.

Included Fixes

Snippet editor and snippet data

  • Restored snippet description rendering in both:
    • the snippet editor view
    • the All Snippets table
  • Fixed Save and Activate for brand new snippets so activation works reliably on the first save.
  • Fixed activation failure handling for snippets with PHP errors:
    • activation does not silently fail
    • the error notice includes the single-line PHP error message and an expandable stack trace
  • Added shared notice styling for traced activation errors, including horizontal scrolling for long stack traces.
  • Removed the cs-back back link from the editor.

Admin navigation fixes

  • Fixed admin bar snippet links to use the correct id query parameter.
  • Changed the Edit Snippet sidebar item so it no longer navigates away from the current page:
    • clicking it focuses the CodeMirror editor

Export and download fixes

  • Fixed bulk export so selected snippets can be exported again as JSON for re-import.
  • Kept Export and Download as separate actions:
    • Export produces importable JSON
    • Download produces raw snippet files
  • Added bulk-only Download action for snippet code:
    • single selection downloads as an individual file based on snippet type
    • multiple selections download as a ZIP archive when ZipArchive is available
  • Adjusted the bulk action UI so:
    • Apply is disabled by default and while Bulk actions is selected
    • Apply enables when a real action is selected

Flat Files compatibility

  • Fixed the Flat Files + LiteSpeed Cache fatal by preventing early bootstrap code from calling unavailable WordPress functions.

Manage Screen Improvements

Screen Options

  • Added Screen Options support for choosing visible columns on the All Snippets table.
  • Added a Table Options section for snippets table behavior with a toggle for truncating long row values.
  • Restricted Screen Options so:
    • snippets view shows Table Options, Columns, and Pagination
    • cloud-community shows only Pagination

Table behavior and markup

  • Fixed sortable header markup/classes to match WordPress core sorted column behavior.

Community Cloud pagination

  • Applied the Screen Options pagination setting to Community Cloud requests passing per_page through the local REST layer and outbound cloud API request.
  • Capped cloud per_page at 100 even if the user’s screen option is higher (avoids api error messages)

imantsk added 30 commits March 10, 2026 12:21
@imantsk imantsk added the build Adding this label will trigger the zip build action label Mar 11, 2026
const getVisibleSelected = <T, K extends Key>(visibleItems: T[], getKey: (item: T) => K, selected: Set<K>): Set<K> =>
new Set(visibleItems.map(getKey).filter(key => selected.has(key)))

const buildTableNavProps = <K extends Key>({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of this function? It seems to just be passing values through directly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the redundant pass-through helper and inlined the props construction.
Commit: 72b6b0db1

<TableNav which="bottom" {...tableNavProps} />
</>
)
return <ListTableMarkup
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the benefit in taking this out separately?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed the unnecessary extraction
Commit: 72b6b0db1

? 'asc' === sortDirection ? 'desc' : 'asc'
: column.defaultSortDirection ?? 'asc'
const classDirection = isCurrent ? sortDirection : 'asc' === nextSortDirection ? 'desc' : 'asc'
const ariaSort = isCurrent ? 'asc' === sortDirection ? 'ascending' : 'descending' : undefined
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be inlined.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed
Commit: b8e442ad5


const TableCell = <T, >({ item, column }: TableCellProps<T>) => {
const className = `${column.id}-column column-${column.id}`
const className = [ `${column.id}-column`, `column-${column.id}`, column.isHidden ? 'hidden' : '' ]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the classnames.classnames module for this sort of thing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed
Commit: 19ed1ea50

id={`bulk-action-selector-${which}`}
value={selectedActionName}
onChange={event => {
setSelectedActionName(event.target.value)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit of a shame to lose the typing here – what's the benefit in this change vs what we are losing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed
Commit: ecfa584fe

return 'snippets_per_page' === $option ? $value : $status;
}

/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be handled through the REST API, IMO.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

* @return void
*/
public function sync_active_shared_network_snippets_add( $option, $value ): void {
if ( 'active_shared_network_snippets' !== $option ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

huh??

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewrote the option handler with a clearer early-return guard style.
Commit: 0c70958f4

*/
public static function get_hashed_table_name( string $table ): string {
return wp_hash( $table );
return function_exists( 'wp_hash' ) ? wp_hash( $table ) : md5( $table );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wp_hash has been around since 2.0.3 – do we really need to assume it may not exist? Or is it a contextual fill-in?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it’s because of load order and namespacing, not as much about WP version support.
Plugin like LiteSpeed can trigger this path before pluggable functions are loaded, and in a namespace we must call \wp_hash. The guard/fallback is to avoid a fatal during early bootstrap; once WP is fully loaded we’ll use \wp_hash as normal.
Commit: b9ca72a66

*/
protected static function resolve_field_name( string $field ): string {
return self::$field_aliases[ $field ] ?? $field;
return static::$field_aliases[ $field ] ?? $field;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great pick-up, thanks.

@@ -755,6 +763,8 @@ function execute_snippet( string $code, int $id = 0, bool $force = false ) {
$result = eval( $code );
} catch ( ParseError $parse_error ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might as well remove the ParseError catch if we're going to be catching Throwable anyway.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed redundant ParseError catches and the unused import.
Commit: c11d81954

@imantsk
Copy link
Member Author

imantsk commented Mar 16, 2026

@sheabunge
ref: #337 (comment)
ref: #337 (comment)

REST downloads are possible (set Content-Disposition: attachment and return raw bytes), but in WP they tend to be more fragile in practice: REST is JSON-first, fetch won’t trigger a native browser download without extra client-side Blob/link work, and large ZIP streaming can run into buffering/memory edge cases. For Screen Options specifically, if we want the controls to live in WP’s native Screen Options flyout (columns/per-page), we’re constrained to core’s PHP hooks (add_screen_option, screen_settings, set-screen-option) and then have React consume/apply the persisted values; moving the UI itself “into React” would mean building and maintaining a separate custom settings UI rather than using native Screen Options.

@imantsk imantsk added build Adding this label will trigger the zip build action and removed build Adding this label will trigger the zip build action labels Mar 16, 2026
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.

2 participants