-
Notifications
You must be signed in to change notification settings - Fork 15k
fix(training): mobile spacing and width #52398
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
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
✅ Pull request preview available for checkingBuilt without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
I want to make sure I understand the rationale, especially around the changes to how we lay out the logos for the individual certifications.
static/css/training.css
Outdated
#iframe-landscape { | ||
flex: 0 1 80%; | ||
max-width: 1200px; | ||
min-width: 0 !important; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we do this without using !important
? For me, that marker should always be a very last resort.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just refactored and pushed. cncf-landscape.html
has both of these elements styled inline so I added the style changes inline there also. So they're no longer in the stylesheet, and goodbye !important
!
body.cid-training section.call-to-action .main-section > div.call-to-action div.cta-image { | ||
padding: 0 2rem 0 2rem; | ||
/* Change display to CSS Grid layout with 2 columns that autofill */ | ||
display: grid; | ||
grid-template-columns: repeat(auto-fill, minmax(50%, 1fr)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this removal? What's the look you're aiming for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My intention was to keep the same original visual intent and structure for the CTA image layout but remove the noticeable offset preventing them from being properly centered on mobile (like it is already on desktop).
This code block was having unintended side-effects given the system's structure, removing it was key.
The padding, even though its values were set to be evenly distributed, was causing the images to be offset on mobile, and removing it in this setup did not have an impact on the desktop layout.
The grid was also unnecessary, upon removing it with the current setup, the image layout is the same as it was, but without the offset on mobile. Adding it back in for my build actually breaks the page, but finding a way to put it in at this point wouldn't make sense when the setup I have creates the same layout for the images as it originally was on all breakpoints. So there's no justification for keeping it (or restoring it from my branch) when it isn't actually adding any meaningful structure and only risks complicating and/or breaking the layout.
The outcome is that the image layout should look pretty much the same on desktop and mobile as it already did but without any offset throwing the centering off.
f4ab682
to
624c727
Compare
- Centered CTA text and images - Expanded CTA text width for better readability - Centered iframe logo grid
624c727
to
00a5512
Compare
Description
There is a related group of mobile formatting issues on the "training" page for three sections including two CTA blocks and a Partnership logo grid iframe. The issues are a lack of consistent centering of elements that are unintentionally offset (including the logo grid hugging the left page margin) and an overly narrow width constraining the CTA text elements affecting page balance and readability.
Solutions
training.css
and inline style forcncf-landscape.html
; no content is modified.Screenshots
CTA - Section 1
Before

After

CTA - Section 2
Before

After

iframe Logo Grid - Section 3
Before

After
