Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix errors and warnings in the course/lesson/question editor #7646

Merged
merged 23 commits into from
Aug 6, 2024
Merged

Fix errors and warnings in the course/lesson/question editor #7646

merged 23 commits into from
Aug 6, 2024

Conversation

renatho
Copy link
Contributor

@renatho renatho commented Jul 29, 2024 •
edited
Loading

Part of #7624

Proposed Changes

  • Fix JS errors and warnings displayed in the browser console.
  • Since it's just fixing deprecated code, and users won't see any change, I'm not adding any changelog.

Known issues

  • When navigating to the course editor, and navigating back (browser history), it gives the error Blocked attempt to show a 'beforeunload' confirmation panel for a frame that never had a user gesture since its load.. I could isolate it noticing that the warning is not displayed anymore when we remove the TextControl from the modal.
    • I didn't invest much time into that because it's external and not too critical.
  • Deprecated prop on Tour Kit wp-calypso#93167 - When using the tour, there is a warning about a deprecated prop (isElevated), which comes from Calypso.

Testing Instructions

  1. Prepare 2 development environments and repeat the test for WordPress 6.4 and WordPress 6.6.
  2. For WordPress 6.6, activate Gutenberg with the version from trunk or:
    2.1. Install the WordPress Beta Tester plugin.
    2.2. Switch to the bleeding edge nightly.
  3. Open the browser console and monitor for warnings and errors.
  4. While performing the following tests, play with the blocks settings and toolbar.
  5. Create a course with lessons.
  6. In a lesson, create a quiz with questions.

Pre-Merge Checklist

  • PR title and description contain sufficient detail and accurately describe the changes
  • Acceptance criteria is met
  • Decisions are publicly documented
  • Adheres to coding standards (PHP, JavaScript, CSS, HTML)
  • All strings are translatable (without concatenation, handles plurals)
  • Follows our naming conventions (P6rkRX-4oA-p2)
  • Hooks (p6rkRX-1uS-p2) and functions are documented
  • New UIs are responsive and use a mobile-first approach
  • New UIs match the designs
  • Different user privileges (admin, teacher, subscriber) are tested as appropriate
  • Legacy courses (course without blocks) are tested
  • Code is tested on the minimum supported PHP and WordPress versions
  • User interface changes have been tested on the latest versions of Chrome, Firefox and Safari
  • "Needs Documentation" label is added if this change requires updates to documentation
  • Known issues are created as new GitHub issues

@renatho renatho added this to the 4.24.2 milestone Jul 29, 2024
@github-actions GitHub Actions
Copy link

Test the previous changes of this PR with WordPress Playground.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 29, 2024 •
edited
Loading

WordPress Dependencies Report

The github-action-wordpress-dependencies-report action has detected some script changes between the commit f38ab70 and trunk. Please review and confirm the following are correct before merging.

Script Handle Added Dependencies Removed Dependencies Total Size Size Diff
js/admin/course-edit.js wp-block-editor, wp-editor 9.87 kB +149 B ( +1.54% 🔼 )
js/admin/lesson-edit.js wp-block-editor, wp-edit-post, wp-editor 2.57 kB +81 B ( +3.26% 🔼 )
home/index.js 21.2 kB +26 B ( +0.13% 🔼 )
data-port/import.js 14.5 kB +111 B ( +0.78% 🔼 )
data-port/export.js 5.46 kB -11 B ( -0.21% ⬇️ )
blocks/single-course.js 30.3 kB +72 B ( +0.24% 🔼 )
blocks/lesson-action-blocks.js 13.6 kB -81 B ( -0.6% ⬇️ )
blocks/global-blocks.js 21.1 kB -44 B ( -0.21% ⬇️ )
blocks/quiz/index.js 34.8 kB +2.12 kB ( +6.48% 🔼 )
blocks/shared.js 13.8 kB -90 B ( -0.65% ⬇️ )
admin/course-pre-publish-panel/index.js wp-editor 4.02 kB +27 B ( +0.68% 🔼 )
admin/editor-wizard/index.js 12.7 kB +45 B ( +0.36% 🔼 )
admin/tour/course-tour/index.js 35.9 kB +18 B ( +0.06% 🔼 )
admin/tour/lesson-tour/index.js wp-editor 36.3 kB -14 B ( -0.04% ⬇️ )
course-theme/blocks/index.js 13.2 kB -29 B ( -0.22% ⬇️ )

This comment was automatically generated by the github-action-wordpress-dependencies-report action.

@renatho renatho changed the title Fix/errors and warnings Fix errors and warnings Jul 29, 2024
@github-actions GitHub Actions
Copy link

Test the previous changes of this PR with WordPress Playground.

@github-actions GitHub Actions
Copy link

Test the previous changes of this PR with WordPress Playground.

@github-actions GitHub Actions
Copy link

Test the previous changes of this PR with WordPress Playground.

renatho
renatho commented Jul 29, 2024
View reviewed changes
assets/admin/course-pre-publish-panel/course-pre-publish-panel.js Outdated
import { ToggleControl } from '@wordpress/components';

/**
* Internal dependencies
*/
import SenseiIcon from '../../icons/logo-tree.svg';

if ( ! PluginPrePublishPanel ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We still need the old ones here for WP 6.4.

renatho
renatho commented Jul 29, 2024
View reviewed changes
assets/admin/tour/data/store.js
@@ -80,11 +81,9 @@ export const reducers = {
DEFAULT: ( action, state ) => state,
};

export const store = createReduxStore( SENSEI_TOUR_STORE, {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The store import is not used anywhere.

@github-actions GitHub Actions
Copy link

Test the previous changes of this PR with WordPress Playground.

@renatho renatho force-pushed the fix/errors-and-warnings branch from c0c0c7c to c82cb2f Compare July 29, 2024 22:26
@renatho renatho self-assigned this Jul 29, 2024
@github-actions GitHub Actions
Copy link

Test the previous changes of this PR with WordPress Playground.

@renatho renatho force-pushed the fix/errors-and-warnings branch from c82cb2f to c594d47 Compare July 29, 2024 22:33
@github-actions GitHub Actions
Copy link

Test the previous changes of this PR with WordPress Playground.

@github-actions GitHub Actions
Copy link

Test the previous changes of this PR with WordPress Playground.

@github-actions GitHub Actions
Copy link

Test the previous changes of this PR with WordPress Playground.

@github-actions GitHub Actions
Copy link

Test the previous changes of this PR with WordPress Playground.

@github-actions GitHub Actions
Copy link

Test the previous changes of this PR with WordPress Playground.

renatho
renatho commented Jul 30, 2024
View reviewed changes
assets/course-theme/blocks/register-template-blocks.js
const isTemplate =
'lesson' === postType && editPost.isEditingTemplate();
toggleBlockRegistration( isTemplate );
toggleBlockRegistration( false );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With the current version of Gutenberg, this has the same effect as the previous code.

See https://github.com/WordPress/gutenberg/blob/060d4ca57ab8441255db34e308c21a640adad2a6/packages/edit-post/src/store/selectors.js#L534-L540

Basically, it was checking 'lesson' === postType && 'wp_template' === postType.

Checking the history of that function, it had some variations, but this should be enough. I tested it in the lesson editor and in the template using WP 6.4 and 6.6, and it seems to be working well (blocks available on the template and not available on the lesson editor).

@github-actions GitHub Actions
Copy link

Test the previous changes of this PR with WordPress Playground.

@github-actions GitHub Actions
Copy link

Test the previous changes of this PR with WordPress Playground.

@github-actions GitHub Actions
Copy link

Test the previous changes of this PR with WordPress Playground.

@github-actions GitHub Actions
Copy link

Test the previous changes of this PR with WordPress Playground.

@github-actions GitHub Actions
Copy link

Test the previous changes of this PR with WordPress Playground.

renatho
renatho commented Jul 31, 2024
View reviewed changes
assets/blocks/quiz/quiz-block/quiz-validation.js
@@ -61,24 +114,7 @@ const IncompleteQuestionsNotice = ( { count, onClick } ) => (
* @param {Function} props.setMeta
*/
const QuizValidationResult = ( { clientId, setMeta } ) => {
const incompleteQuestions = useSelect(
Copy link
Contributor Author

@renatho renatho Jul 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

See https://developer.wordpress.org/news/2024/03/28/how-to-work-effectively-with-the-useselect-hook/#be-careful-about-transforming-data-inside-the-callback

We caught this as part of this PR because it was also causing a warning.

The full commit which also needed to memorize a selector: 993b12a

@github-actions GitHub Actions
Copy link

Test the previous changes of this PR with WordPress Playground.

@renatho renatho force-pushed the fix/errors-and-warnings branch from 8bb0f60 to 993b12a Compare July 31, 2024 20:29
@renatho renatho force-pushed the fix/errors-and-warnings branch from 3eefd28 to db0060f Compare July 31, 2024 21:22
@github-actions GitHub Actions
Copy link

Test the previous changes of this PR with WordPress Playground.

@renatho renatho marked this pull request as ready for review July 31, 2024 21:36
@renatho renatho requested a review from a team July 31, 2024 21:36
@renatho renatho changed the title Fix errors and warnings Fix errors and warnings in the course/lesson/question editor Jul 31, 2024
@renatho renatho mentioned this pull request Jul 31, 2024
Open
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 1, 2024

Test the previous changes of this PR with WordPress Playground.

m1r0
m1r0 reviewed Aug 1, 2024
View reviewed changes
Copy link
Member

@m1r0 m1r0 left a comment

Choose a reason for hiding this comment

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

Thanks for working on this, Renatho!

After testing, I've noticed just one warning related to the Course Actions block:

AM9z11.mp4

Overall it looks great but I didn't have time to test everything. I'll continue on Monday if no one picks this up.

@renatho
Copy link
Contributor Author

renatho commented Aug 2, 2024

Good catch, Miro! Fixed this one too! f38ab70

@renatho renatho requested a review from m1r0 August 2, 2024 17:28
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 2, 2024

Test the previous changes of this PR with WordPress Playground.

donnapep
donnapep approved these changes Aug 6, 2024
View reviewed changes
Copy link
Collaborator

@donnapep donnapep left a comment

Choose a reason for hiding this comment

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

Tested and it works OK for me. I still see this in the console, but it's specific to the Course theme so I'll open a new issue for that:

Screenshot 2024-08-06 at 12 18 17 PM

@renatho renatho merged commit 71b1a9d into trunk Aug 6, 2024
23 checks passed
@renatho renatho deleted the fix/errors-and-warnings branch August 6, 2024 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@donnapep donnapep donnapep approved these changes

@m1r0 m1r0 Awaiting requested review from m1r0

Assignees

@renatho renatho

Labels
No Changelog
Projects
None yet
Milestone
4.24.2
Development

Successfully merging this pull request may close these issues.
3 participants
@renatho @donnapep @m1r0