-
Notifications
You must be signed in to change notification settings - Fork 376
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
feat: develop Tetragon Use Cases pages #3277
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for tetragon ready!
To edit notification comments on pull requests, go to your Netlify site 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.
Hey thanks Anna this looks good, I'll review it quickly, could you maybe split this into the logical commits you described in the PR description?
- Developed 7 Use Case Pages
- Added a new dropdown to the Header
- Refactored the Conference Talks table. Moved video data to docs/data in YAML for reuse
- Moved the social section to a partial for easier reuse in layouts
This would make 4 nice commits I imagine and would be easier to review.
By the way, nice initiative with "Refactored the Conference Talks table. Moved video data to docs/data in YAML for reuse", this looks great like that, that's a very good idea.
When on your working branch, you can easily do git reset HEAD~
and then git add
the appropriate parts and git commit -s
so that you create 4 separate commits. That would be great, thanks.
Feel free to ask again for review with the GH button when done.
d9e78c0
to
5ad45ea
Compare
Signed-off-by: Anna Artemova <[email protected]>
Signed-off-by: Anna Artemova <[email protected]>
Signed-off-by: Anna Artemova <[email protected]>
5ad45ea
to
3104ab5
Compare
Signed-off-by: Anna Artemova <[email protected]>
3104ab5
to
26295b7
Compare
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.
From a technical PoV on the docs, this change looks good to me! Thanks, this is really well done.
I would just wait for Paul to look at the use cases since I think it's the one that initiated this and I didn't look at the content.
.td-navbar-nav-scroll { | ||
height: auto; | ||
} | ||
.navbar-nav { | ||
padding-bottom: 0; | ||
overflow: hidden; | ||
} | ||
} | ||
|
||
@media screen and (max-width: 767px) { |
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.
curious: why did we needed to remove this media size and change the other from 767 to 991.98px?
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.
Never a super fan to get away from the original theme, but since this doesn't exist this is the only solution. It seems the community has been asking for it google/docsy#311 if you ever want to upstream this and do some OSS work :). You can see another person's version here google/docsy#311 (comment).
[[menu.navbar_main]] | ||
name = "Capabilities Monitoring" | ||
parent = "use-cases" | ||
url = "/use-cases/capabilities-monitoring" | ||
weight = 1 |
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.
It looks like your version looks great and I have not dive into the technical details, so maybe ignore this totally: but I was wondering if it would have been possible to do something like [[menu.navbar_main.use-cases]]
and then declare each use-cases using this.
@@ -478,7 +478,7 @@ <h2 class="title videos__title"> | |||
<ul class="videos__list"> | |||
<li> | |||
<a href="https://www.youtube.com/watch?v=u8HKg5pENj4" target="_blank" rel="noopener noreferrer"> | |||
<img src="/images/cover-log4shell.jpg" width="640" height="360" loading="lazy" alt="youtube video cover of log4shell presentation"> | |||
<img src="/images/video-preview/the-next-log4jshell.jpg" width="640" height="360" loading="lazy" alt="youtube video cover of log4shell presentation"> |
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.
nit: indentation looks off
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.
ah nevermind, this is because you change it in a later commit, ignore this :)
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 this is really great!
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 must say I'm impressed on how you made those use cases easy to write: you really use Hugo here to generate those pages and that's cool! Thanks.
Describe what changes this pull request brings
docs/data
in YAML for reuseNOTE: the video section on the main page hasn’t been updated to use shared data. Let me know if you’d like me to adjust it to show the last 3 videos from table with the current design
Steps to check:
Open preview:
Make sure that everything looks and works as expected