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

Fix formatting of Course List block on course archive page #7180

Merged
merged 2 commits into from
Sep 20, 2023

Conversation

donnapep
Copy link
Collaborator

@donnapep donnapep commented Sep 20, 2023

Resolves #7144.

Proposed Changes

Before we had the Course List block, we enabled people to use a grid layout for the course archive page by using the sensei_course_loop_number_of_columns filter. However, the CSS used for this wasn't specific enough and ended up leaking into other pages, namely those containing the Course List block.

Note that this is only a problem if the page containing the Course List block is set as the Course Archive Page in the settings. If not, it works just fine.

This PR wraps the columns CSS inside the .course-container element, so that it doesn't affect other pages.

Testing Instructions

  1. Create a page containing the Course List block. Ensure you choose a grid pattern.
  2. Set the page as the Course Archive Page in Sensei settings.
  3. View the page on the frontend and ensure it's not squished.
  4. Switch to the Astra theme (just because the legacy grid works reasonably well on Astra).
  5. Create an empty page and set it as the Course Archive Page in Sensei settings.
  6. Hook into the sensei_course_loop_number_of_columns filter and set it to 3 (or just change it in the code).
  7. View the course archive page. It should have a 3 column layout (although it might not look that great).

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
  • 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

Wrap columns CSS inside `.course-container` element.
@donnapep donnapep added this to the 4.17.1 milestone Sep 20, 2023
@donnapep donnapep self-assigned this Sep 20, 2023
@donnapep donnapep changed the title Fix formatting of Course List block on non-course archive page Fix formatting of Course List block on course archive page Sep 20, 2023
@codecov
Copy link

codecov bot commented Sep 20, 2023

Codecov Report

Merging #7180 (d430785) into trunk (80a5cdd) will not change coverage.
Report is 1 commits behind head on trunk.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##              trunk    #7180   +/-   ##
=========================================
  Coverage     49.85%   49.85%           
  Complexity    10651    10651           
=========================================
  Files           586      586           
  Lines         45049    45049           
  Branches        402      402           
=========================================
  Hits          22458    22458           
  Misses        22264    22264           
  Partials        327      327           

Continue to review full report in Codecov by Sentry.

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

@donnapep donnapep requested a review from a team September 20, 2023 14:12
Copy link
Contributor

@Imran92 Imran92 left a comment

Choose a reason for hiding this comment

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

Fixes the squishing as expected and the legacy course grid still works as it used to do 🚀

@donnapep donnapep merged commit cdc78f9 into trunk Sep 20, 2023
24 checks passed
@donnapep donnapep deleted the fix/course-list-width branch September 20, 2023 14:54
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.

Course list block width shrunk in Course page for grid view
2 participants