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

Create and register the patterns for course list block #5433

Merged
merged 10 commits into from
Aug 22, 2022

Conversation

Imran92
Copy link
Contributor

@Imran92 Imran92 commented Aug 9, 2022

Implements #5423

Changes proposed in this Pull Request

  • Added 6 patterns for course list block

Testing instructions

  • Add a Course List block in the block editor
  • Click on 'Choose' to open the pattern selection modal
  • Make sure along with the existing patterns, 6 new patterns are there as per our design. 3 of them should be of Grid structure, 3 of them should be of list structure.

Course Signup and Course Progress block is not currently available in the Page editor. Making the Course Progress block available in the Page editor is in progress and we'll replace the Course Signup block with a new one as part of our upcoming tasks. So for now it's better to test these patterns in the course editor GB page

image

Screenshot / Video

Screen.Recording.2022-08-10.at.2.51.02.AM.mov

@Imran92 Imran92 requested a review from a team August 9, 2022 21:22
@Imran92 Imran92 self-assigned this Aug 9, 2022
@Imran92 Imran92 linked an issue Aug 9, 2022 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Aug 9, 2022

Codecov Report

Merging #5433 (8d586cd) into feature/courses-list (f3b6df3) will decrease coverage by 0.03%.
The diff coverage is 0.00%.

❗ Current head 8d586cd differs from pull request most recent head 63b9f2b. Consider uploading reports for the commit 63b9f2b to get more accurate results

Impacted file tree graph

@@                    Coverage Diff                     @@
##             feature/courses-list    #5433      +/-   ##
==========================================================
- Coverage                   44.67%   44.63%   -0.04%     
+ Complexity                   8779     8755      -24     
==========================================================
  Files                         419      419              
  Lines                       31320    31295      -25     
  Branches                      239      239              
==========================================================
- Hits                        13991    13970      -21     
+ Misses                      17155    17151       -4     
  Partials                      174      174              
Impacted Files Coverage Δ
...des/block-patterns/class-sensei-block-patterns.php 27.69% <0.00%> (+1.22%) ⬆️
...e-list/class-sensei-course-list-block-patterns.php 0.00% <0.00%> (ø)
includes/blocks/class-sensei-block-take-course.php 73.33% <ø> (ø)
includes/class-sensei-admin.php 22.03% <0.00%> (-0.12%) ⬇️
includes/class-sensei-quiz.php 59.18% <0.00%> (ø)
includes/class-sensei-course-structure.php 91.19% <0.00%> (ø)
...urse-theme/class-sensei-course-theme-templates.php 0.00% <0.00%> (ø)
...ssets/shared/helpers/player/round-with-decimals.js
includes/admin/class-sensei-learner-management.php 2.23% <0.00%> (+0.01%) ⬆️
includes/class-sensei-lesson.php 38.58% <0.00%> (+0.07%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3b6df3...63b9f2b. Read the comment docs.

Copy link
Contributor

@gabrielcaires gabrielcaires left a comment

Choose a reason for hiding this comment

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

@Imran92 I am seeing it:
Screen Shot 2022-08-10 at 08 55 05

I think we are missing some styling (Example: There is a shadow on each course DIV/Block)

Is the idea to have the other cards update the patterns? Example: When I finished the course categories, should I update the patterns as part of my card?

@Imran92
Copy link
Contributor Author

Imran92 commented Aug 10, 2022

@Imran92 I am seeing it: Screen Shot 2022-08-10 at 08 55 05

I think we are missing some styling (Example: There is a shadow on each course DIV/Block)

Is the idea to have the other cards update the patterns? Example: When I finished the course categories, should I update the patterns as part of my card?

You're right, we'll have other cards to update the patterns once the Category and Course Action block is done. Making the "Course Signup" block available for pages will be counterproductive for now as we'll have to remove it again later. Can you kindly test the block in the Course editor for now? I'm updating the testing instructions to mention it there

@donnapep
Copy link
Collaborator

@Imran92 Course progress is ready in #5443 and Course Actions block is ready in #5430. I think you will need the Course Actions PR before you'll be able to finish this one. Perhaps we could get course progress merged first (it's a quick review) and then rebase this one against the Course Actions PR?

@gabrielcaires gabrielcaires self-requested a review August 15, 2022 14:13
@Imran92
Copy link
Contributor Author

Imran92 commented Aug 22, 2022

@Imran92 Course progress is ready in #5443 and Course Actions block is ready in #5430. I think you will need the Course Actions PR before you'll be able to finish this one. Perhaps we could get course progress merged first (it's a quick review) and then rebase this one against the Course Actions PR?

Hi @donnapep , all the blocks are now added to our patterns. Can you kindly take a look? Thanks a million :)

@donnapep
Copy link
Collaborator

@Imran92 Looks like it needs a rebase. 🙂

@Imran92
Copy link
Contributor Author

Imran92 commented Aug 22, 2022

@donnapep thanks, I've merged it with the latest feature branch

@donnapep
Copy link
Collaborator

I did a quick test and am only seeing 3 of the patterns load. There are empty boxes for the rest:

Screen Shot 2022-08-22 at 4 04 51 PM

@Imran92
Copy link
Contributor Author

Imran92 commented Aug 22, 2022

Good catch @donnapep ! Fixed it here 63b9f2b, can you kindly do a pull? Thank you ☺️

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.

🥳 🎉 💯 🥇

@Imran92 Imran92 dismissed gabrielcaires’s stale review August 22, 2022 20:15

Addressed and updated in code

@Imran92 Imran92 removed the request for review from gabrielcaires August 22, 2022 20:15
@Imran92 Imran92 merged commit 467c429 into feature/courses-list Aug 22, 2022
@Imran92 Imran92 deleted the add/course-list-patterns branch August 22, 2022 20:20
@Imran92 Imran92 added this to the 4.6.3 milestone Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create patterns for our Course list block
3 participants