-
Notifications
You must be signed in to change notification settings - Fork 1
WIP: Make the editor sidebar show up in the site-editor #746
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
Conversation
|
Test merged PR on Playground |
✅ Code Coverage Report
🎉 Great job maintaining/improving code coverage! 📊 File-level Coverage Changes (1 files)📈 Coverage Improved
ℹ️ About this report
|
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.
Pull request overview
This PR enables the Progress Planner editor sidebar to work in the WordPress site editor by removing a PHP-level bailout and adding JavaScript-level detection to hide the sidebar only when editing templates (wp_template/wp_template_part). The changes include extensive defensive programming improvements to handle edge cases with null values, NaN results, and array operations.
Changes:
- Removed PHP server-side check that prevented script loading in site-editor.php
- Added JavaScript client-side detection to hide sidebar when editing templates (not posts/pages)
- Implemented defensive null/undefined checks and NaN handling throughout the codebase
- Added backwards compatibility layer for openGeneralSidebar API across WordPress versions
- Fixed array handling to prevent empty string elements and avoid mutation of original data
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| classes/admin/class-editor.php | Removed bailout condition that prevented editor script from loading on site-editor.php page |
| assets/js/editor.js | Updated dependency from wp-edit-post to wp-editor, added template detection logic to conditionally render sidebar, extensive defensive programming improvements for null safety and array handling, added API compatibility layer for WordPress 6.6+ |
Comments suppressed due to low confidence (1)
assets/js/editor.js:68
- Similar to the issue with 'progressPlannerEditor.defaultPageType', this code accesses 'progressPlannerEditor.pageTypes' without verifying that 'progressPlannerEditor' exists. Add a check for 'typeof progressPlannerEditor !== "undefined"' at the beginning of the function to prevent potential ReferenceErrors.
// Bail early if the page types are not set.
if (
! progressPlannerEditor.pageTypes ||
0 === progressPlannerEditor.pageTypes.length
) {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (2)
assets/js/editor.js:86
- The early return check for
progressPlannerEditor.pageTypeson lines 81-86 happens after theuseSelecthook on lines 72-78. According to the Rules of Hooks, all hooks must be called in the same order on every render. This early return should be moved before theuseSelectcall, or the check should be done differently to avoid conditional hook execution.
// Bail early if the page types are not set.
if (
! progressPlannerEditor.pageTypes ||
0 === progressPlannerEditor.pageTypes.length
) {
return el( 'div', {}, '' );
}
assets/js/editor.js:228
- The early return check on lines 222-228 happens after two
useSelecthooks on lines 207 and 214. According to the Rules of Hooks, all hooks must be called in the same order on every render. Early returns that happen after hooks can cause issues. The conditional check should be moved after all hook calls, or the check should be done inside the hook callbacks to maintain proper hook execution order.
// Bail early if the page type or lessons are not set.
// Check if progressPlannerEditor exists before accessing its properties.
if (
! pageType ||
typeof progressPlannerEditor === 'undefined' ||
! progressPlannerEditor.lessons ||
0 === progressPlannerEditor.lessons.length
) {
return el( 'div', {}, '' );
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Context
Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
Test instructions
Relevant test scenarios
UI changes
Documentation
Quality assurance
Fixes #