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.

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

Add patterns filter by post type #5202

Merged

Conversation

renatho
Copy link
Contributor

@renatho renatho commented May 27, 2022

Fixes #5187

Changes proposed in this Pull Request

  • After a deeper investigation of the mentioned issue, it seems safe to use the current_screen hook to register the patterns based on the current post type. It wasn't really working on some beta versions, but it was fixed after the final release with this commit (basically, the server adds our registered patterns to the initial setup of the patterns - writing it in the HTML, and the other come later in the async fetch). Anyway, I asked for a confirmation here just to make sure I'm not missing anything.

Testing instructions

  • In WP 6.0, create a new Course, and make sure only 3 patterns are listed in the patterns.
  • Create a new Lesson, and make sure the other 3 patterns are listed in the patterns.
  • It's nice to also test in WP 5.8 and 5.9.

@renatho renatho requested a review from a team May 27, 2022 19:30
@renatho renatho changed the base branch from trunk to feature/course-lesson-wizard May 27, 2022 19:30
@renatho renatho self-assigned this May 27, 2022
@renatho renatho added this to the Course and Lesson Wizard milestone May 27, 2022
@renatho renatho marked this pull request as ready for review May 27, 2022 19:35
aaronfc
aaronfc previously approved these changes May 30, 2022
Copy link
Contributor

@aaronfc aaronfc left a comment

Choose a reason for hiding this comment

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

Looks good and works well! Some things I noticed with older WP versions but I think none are really important and we can "ignore" them being only minor visual things and for old WP versions.

Using WP 5.9.3

  • Some previews are displayed with an excessively long height. Example:

image

Using WP 5.8.4

  • Some previews are not displayed (not only for Sensei LMS but for all categories). Examples:

image

image

image

  • Minor: Modal does not have a cross to close it. Clicking out of the modal or pressing escape works.

image

@renatho
Copy link
Contributor Author

renatho commented May 30, 2022

Hey @aaronfc ! Thank you for your review!

Some previews are displayed with an excessively long height.

I had noticed it and registered it as a known issue in: #5173. I mentioned that we could wait until the final patterns to see if it will happen, and if yes, we could explore that to see what workaround we could implement. One idea I had added there is set a maximum height to the thumbnails. Does it make sense for you? To make sure we don't forget it, I'm also adding it to a comment in this issue.

Some previews are not displayed (not only for Sensei LMS but for all categories)

I thought it had gone away. 😞 Reopened this issue.

Modal does not have a cross to close it.

Good catch! Fixed in f22a70a

@renatho renatho requested a review from aaronfc May 30, 2022 14:41
Copy link
Contributor

@aaronfc aaronfc left a comment

Choose a reason for hiding this comment

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

Everything clear!

Looks good and works well! 🚀

@renatho renatho merged commit fee4534 into feature/course-lesson-wizard May 30, 2022
@renatho renatho deleted the add/patterns-filter-by-post-type branch May 30, 2022 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filter the patterns based on the current post type
2 participants