-
Notifications
You must be signed in to change notification settings - Fork 14.8k
Make the video section height on the main page flexible #49443
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
Signed-off-by: Dmitry Shurupov <[email protected]>
[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 site configuration. |
@natalisucks hi! Can I please ask for your assistance in moving forward with this PR? 🙏 Previously, I made a thread in Slack to get more attention, but it did not help. |
@shurup Thanks for this PR! This is a significant change for docs, and while we do need the mobile bit, it can be iterated upon in a follow-up PR. However, I'd like other tech leads, co-chairs, or any approver to chime in here with their views, as well. |
LGTM label has been added. Git tree hash: bd8df8c73a417eb3379931519b34407f12e4563f
|
This PR improves the KubeCon links section redesign introduced in #49167. While propagating these recent changes to various localisations, I stumbled upon an issue in #49442. When we have a lot of content in the video block (which includes the KubeCon links section), some links might not fit due to a limited height. Here, you can see a missing (hidden) link to the 5th event in the Portuguese localisation:
To prevent this from happening, this PR gets rid of fixed heights for the video block. With
overflow:hidden
added, it makes its height flexible. E.g., we'll be able to add even more events with no issue:To keep backward compatibility with the pages using old KubeCon links, the
#video:has(#desktopKCButton)
workaround is added to_base.scss
.My changes also remove an unnecessary
200px
-height video block with no content for the mobile layout — it will just disappear. Let me know if we really need it back for mobiles:P.S. I tested the changes in two browsers (Chrome & Firefox) and various localisations (e.g., those that have already adopted new KubeCon links and those that have not).