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

Navigation: Add header menu items from website #534

Merged
merged 3 commits into from
Oct 15, 2024
Merged

Conversation

matkuliak
Copy link
Contributor

@matkuliak matkuliak commented Oct 2, 2024

Summary of the changes / Why this is an improvement

We want to bring the top navigation from main page to the docs:

  • Product
  • Use cases
  • Industries
  • Resources
  • Pricing

Preview

https://crate-docs-theme--534.org.readthedocs.build/en/534/

image


cc @geragray

@cla-bot cla-bot bot added the cla-signed label Oct 2, 2024
@matkuliak matkuliak force-pushed the mm/top-navigation-edit branch 3 times, most recently from dd9dd5e to 67f0fe0 Compare October 2, 2024 16:45
@matkuliak matkuliak changed the title Navigation: Add items from website Navigation: Add header menu items from website Oct 2, 2024
@matkuliak matkuliak force-pushed the mm/top-navigation-edit branch 3 times, most recently from 1b354c8 to c6f66a7 Compare October 2, 2024 19:35
@matkuliak
Copy link
Contributor Author

matkuliak commented Oct 2, 2024

Hi @msbt, rough draft of the navigation similar to our main site. I'm not really familiar with all the css in our docs, it's split into a couple of files so I mostly made new classes. Let me know if I should shuffle some stuff elsewhere. Thanks!

@matkuliak matkuliak requested a review from msbt October 3, 2024 06:56
@matkuliak matkuliak self-assigned this Oct 3, 2024
@matkuliak matkuliak requested a review from amotl October 3, 2024 08:14
@amotl
Copy link
Member

amotl commented Oct 3, 2024

Thanks. Isn't this supposed to go into some SSI snippet?

@matkuliak
Copy link
Contributor Author

matkuliak commented Oct 3, 2024

Not sure. After pushing, @msbt let me know about https://cratedb.com/_hcms/api/navi-header. That was previously used as a dynamic source for the list through ESI. So going to switch to that probably

@amotl
Copy link
Member

amotl commented Oct 8, 2024

Can you add this to https://cratedb.com/_hcms/api/navi-header instead, @msbt? 🙏

@amotl amotl marked this pull request as draft October 8, 2024 19:42
@msbt
Copy link
Collaborator

msbt commented Oct 10, 2024

@amotl I'll give it a go, might take some time though

@msbt
Copy link
Collaborator

msbt commented Oct 14, 2024

@amotl I've implemented the current navigation as fallback and reintroduced the esi's, are we happy with it? PR link is to the latest status (https://crate-docs-theme--534.org.readthedocs.build/en/534/)

@geragray geragray assigned geragray and unassigned matkuliak Oct 15, 2024
@amotl amotl requested a review from geragray October 15, 2024 15:43
Copy link
Member

@amotl amotl left a comment

Choose a reason for hiding this comment

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

Thanks. Please check, @geragray and/or @kneth. Thanks!

@amotl amotl marked this pull request as ready for review October 15, 2024 15:44
@amotl
Copy link
Member

amotl commented Oct 15, 2024

I guess it will be good to merge and release, and applying "ack by timeout" will not cause too much harm, even despite its duration was a bit short here. If other reviewers can spot any annoyances, or wish to add more items, relevant suggestions will be considered in retrospective.

@amotl amotl merged commit 2ba305d into main Oct 15, 2024
9 checks passed
@amotl amotl deleted the mm/top-navigation-edit branch October 15, 2024 21:05
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.

4 participants