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

Update navigation.yml #780

Merged
merged 14 commits into from
Mar 24, 2025
Merged

Update navigation.yml #780

merged 14 commits into from
Mar 24, 2025

Conversation

colleenmcginnis
Copy link
Contributor

Copy link
Contributor Author

@colleenmcginnis colleenmcginnis left a comment

Choose a reason for hiding this comment

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

Some specific questions below!

Comment on lines 302 to 306

# This makes sense for now. I'm not sure when these files will be
# moved to https://github.com/elastic/detection-rules.
- toc: security-docs://reference/prebuilt-rules
path_prefix: reference/prebuilt-rules
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can be moved up to replace lines 161-162.

Copy link
Member

Choose a reason for hiding this comment

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

@colleenmcginnis
Copy link
Contributor Author

FYI @Mpdreamz we will need to merge elastic/docs-content#792 to get the final structure of the global nav.

Copy link
Contributor Author

@colleenmcginnis colleenmcginnis left a comment

Choose a reason for hiding this comment

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

Ok. I think this is pretty stable now. I left a few comments below.

Comment on lines +76 to 77
logstash-docs-md:
skip: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This content is still in flux.

@colleenmcginnis colleenmcginnis marked this pull request as ready for review March 21, 2025 18:35
@colleenmcginnis colleenmcginnis requested a review from a team as a code owner March 21, 2025 18:35
@colleenmcginnis
Copy link
Contributor Author

Hey @colleenmcginnis you might need to delete your .artifacts/checkouts folder and call clone-all again.

Alternatively you should be able to call clone-all without deleting to update all but I have not tested if it works in all scenario's yet.

Thanks! I tried both and continued to see errors when building locally so I haven't deleted these items yet. Let me know if I should delete them anyway or feel free to merge this whenever you're ready for it!

@@ -417,13 +396,13 @@ toc:
# ECS
# ✅ https://github.com/elastic/docs-content/blob/main/reference/ecs/toc.yml
- toc: docs-content://reference/ecs
path_prefix: reference/ecs
path_prefix: reference/ecs/overview
Copy link
Contributor Author

@colleenmcginnis colleenmcginnis Mar 24, 2025

Choose a reason for hiding this comment

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

I did this for now for ecs, cloud, and elasticsearch. We might want to move the landing page (index.md file) from docs-content to the source repos, but I think the fix would take a while to implement because we would need to update any links that point to those files because I don't think there's a way to redirect to a page in a different repo, right?

Copy link
Member

Choose a reason for hiding this comment

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

That is correct!

@colleenmcginnis
Copy link
Contributor Author

Ok. I have the left nav building as I expected, but when I click on some of the links in the nav I get "page can't be found" this is (mostly?) just pages coming from the elasticsearch repo. Maybe we need to update the docset.yml in that repo with the same treatment we just gave docs-content?

page-cant-be-found.mov

@Mpdreamz
Copy link
Member

@colleenmcginnis I believe so.

I will need to add validation that toc.yml links are not deeplinks into folders.

@colleenmcginnis
Copy link
Contributor Author

Does this look right? elastic/elasticsearch#125509

colleenmcginnis and others added 2 commits March 24, 2025 14:27
Simplified navigation logic by restructuring method signatures and refining table of contents processing. Added support for phantom definitions in navigation files, ensuring appropriate handling of unlinked TOC folders. Refactored related classes to align with the new navigation paradigm.
@colleenmcginnis
Copy link
Contributor Author

0 errors 🎉

image

@colleenmcginnis colleenmcginnis enabled auto-merge (squash) March 24, 2025 20:58
@colleenmcginnis colleenmcginnis merged commit a1a1870 into main Mar 24, 2025
10 checks passed
@colleenmcginnis colleenmcginnis deleted the update-nav branch March 24, 2025 21:10
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.

3 participants