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

Course Theme - Fix post title spacings for course and lesson default #7294

Open
wants to merge 9 commits into
base: trunk
Choose a base branch
from

Conversation

Imran92
Copy link
Contributor

@Imran92 Imran92 commented Aug 7, 2023

Changes proposed in this Pull Request:

  • Fixed the spacings above and below the post title of lessons and courses for course theme to match the design.
  • We've changed only spacings specific to the default templates of Course and Lesson, because if any user already changed to another template choosing that style, we don't want to break their existing look. The default look will not be very different either.

Testing instructions:

The top spacing is calculated as the spacing between the bottom of the site title to the top of the post title.

Design

Mobile:
(Top)
Screenshot 2023-08-07 at 5 59 06 PM

(Bottom)
Screenshot 2023-08-07 at 9 46 16 PM

Desktop:
(Top)
Screenshot 2023-08-07 at 9 50 32 PM
(Bottom)
Screenshot 2023-08-07 at 9 50 44 PM

Course Desktop:
Screenshot 2023-08-07 at 5 53 20 PM
Screenshot 2023-08-07 at 5 53 37 PM

Lesson Desktop:
Screenshot 2023-08-07 at 5 56 52 PM
Screenshot 2023-08-07 at 5 57 03 PM

Course Mobile:
Screenshot 2023-08-07 at 5 54 38 PM
Screenshot 2023-08-07 at 5 57 52 PM

Lesson Mobile:
Screenshot 2023-08-07 at 5 57 37 PM
Screenshot 2023-08-07 at 5 57 52 PM

Related issue(s):

#7144

@Imran92 Imran92 added this to the Course - 1.3.1 milestone Aug 7, 2023
@Imran92 Imran92 requested a review from a team August 7, 2023 15:57
@Imran92 Imran92 self-assigned this Aug 7, 2023
@donnapep donnapep modified the milestones: Course - 1.3.1, Course 1.3.2 Aug 9, 2023
@donnapep
Copy link
Contributor

donnapep commented Aug 28, 2023

@Imran92 It looks like there are some conflicts that need to be resolved now. 🙁

Is there an associated issue for this? If not, would you mind adding the Figma file in the description so I can see what the design is supposed to look like? Oops, saw this linked at the very bottom. Maybe it could be linked in the sidebar so I don't miss it next time? 😅

Also, I think we need to change the branch from trunk for this now. 🙂

@Imran92 Imran92 linked an issue Aug 29, 2023 that may be closed by this pull request
@Imran92 Imran92 changed the base branch from trunk to feature/frontend-improvements August 29, 2023 09:53
@Imran92
Copy link
Contributor Author

Imran92 commented Aug 29, 2023

Thanks for taking a look @donnapep !

@Imran92 It looks like there are some conflicts that need to be resolved now. 🙁

Resolved!

Oops, saw this linked at the very bottom. Maybe it could be linked in the sidebar so I don't miss it next time? 😅

Yeah PR template has it at the bottom and also doesn't seem to automatically connect to the issue. Added it on the sidebar now 🤜 🤛

Also, I think we need to change the branch from trunk for this now. 🙂

As it only fixes the spacings of the titles of the lesson and quiz template, I seems pretty safe to me to merge it in trunk. But it'll be a bit too much if one version update gets released only for this small update :P changed the target branch from trunk to frontend-improvements

@@ -448,6 +448,13 @@ body.page-template-page-wide-width footer .wp-block-group {
/*
* Sensei-specific styles
*/
body[class*="-template-single-course"] .wp-block-post-title,
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that the following 2 CSS classes are the ones that style courses and lessons, so I'm wondering which use case this selector is needed for?

@donnapep donnapep modified the milestones: Course 1.3.3, Course 1.3.4 Sep 12, 2023
Base automatically changed from feature/frontend-improvements to trunk September 13, 2023 10:40
@donnapep donnapep modified the milestones: Course 1.3.4, Course 1.3.5 Oct 14, 2023
@donnapep donnapep modified the milestones: Course 1.3.5, Course 1.3.6 Oct 31, 2023
@donnapep
Copy link
Contributor

@Imran92 Can you provide an update here?

@Imran92
Copy link
Contributor Author

Imran92 commented Feb 5, 2024

Hi Donna 👋 I looked into it a bit to figure out what this #7294 (comment) selector was used for, but haven't figured out any. But I'll again try to find it out. In case I don't, I'll remove this particular selector and update this PR

@donnapep donnapep removed this from the Course 1.3.6 milestone Mar 20, 2024
@donnapep donnapep added this to the Course 1.3.7 milestone Mar 20, 2024
@donnapep donnapep removed this from the Course 1.3.7 milestone Apr 25, 2024
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.

Adjust spacing between header and title
2 participants