-
Notifications
You must be signed in to change notification settings - Fork 108
feat: Native Admin Bar QuickNav (Snippets) by DECKERWEB #328
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
base: core-beta
Are you sure you want to change the base?
Conversation
…d safe mode support
fix: better handle null tags in snippet data
Release: v3.9.4
src/js/admin-bar.ts
Outdated
| @@ -0,0 +1,296 @@ | |||
| type PaginationStatus = 'active' | 'inactive' | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make sure that types which are referenced in exported functions are also exported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/js/admin-bar.ts
Outdated
| type PaginationStatus = 'active' | 'inactive' | ||
| type PaginationAction = 'first' | 'prev' | 'next' | 'last' | ||
|
|
||
| type SnippetResponseItem = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Object types like these should ideally be interfaces for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
| if (!config) return '' | ||
|
|
||
| const url = new URL(config.restUrl) | ||
| url.searchParams.set('status', status) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice use of URL.
src/js/admin-bar.ts
Outdated
| } | ||
|
|
||
| const getTypeFromScope = (scope: string): string => { | ||
| if (scope.endsWith('-css')) return 'css' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I generally prefer to avoid if statements without braces – I'd suggest using a switch for this sort of thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refactored to switch + updated ifs to use braces
| $status = sanitize_key( (string) $request->get_param( 'status' ) ); | ||
|
|
||
| if ( in_array( $status, [ 'active', 'inactive' ], true ) ) { | ||
| $all_snippets = array_filter( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be worth combining these array_filter calls into a single iteration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes! combined the array_filter() calls into a single pass
src/php/class-plugin.php
Outdated
| $this->evaluate_functions = new Evaluate_Functions( $this->db ); | ||
|
|
||
| // Admin bar integration. | ||
| require_once $includes_path . '/class-admin-bar.php'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Composer autoloader should handle loading this class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
src/php/class-plugin.php
Outdated
| // Admin bar integration. | ||
| require_once $includes_path . '/class-admin-bar.php'; | ||
| $this->admin_bar = new Admin_Bar(); | ||
| $this->admin_bar->register_hooks(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might as well combine register_hooks into the Admin_Bar constructor; there doesn't seem to be really any value to dividing them like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right! Moved hook registration into Admin_Bar::__construct() and removed the extra register_hooks() call.
…ss hooks directly on load ref: #328 (comment)
…ch and fix lint issues
Summary
This PR brings a native Snippets QuickNav experience into Code Snippets via the WordPress Admin Bar. It adds a dedicated “Snippets” top-level admin bar menu with quick links plus paginated Active/Inactive snippet lists that update dynamically.
Credit
This work is based on the approach and UX established by “Snippets QuickNav” by David Decker – DECKERWEB (GitHub: @deckerweb):
https://github.com/deckerweb/snippets-quicknav
This PR implements the feature natively, following Code Snippets’ existing architecture and coding style.
What’s included
(PHP) My snippet title(type inferred fromtype)https://snipco.de/safe-mode(opens in new tab, uses Dashicons)Settings added
How to test