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

nav: updates #3599

Closed
wants to merge 14 commits into from
Closed

nav: updates #3599

wants to merge 14 commits into from

Conversation

jorgeorpinel
Copy link
Contributor

@jorgeorpinel jorgeorpinel commented May 28, 2022

  • Standard and optimized sidebar.json structure
  • Fix some problems and typos related to the changes

Relates to #144 and prepares the nav for VSCode Extension docs

  • Split into actionable PRs per section + assign to corresponding teams
  • Apply similar changes to MLEM.ai/doc
  • Remove some of oldest redirects too?

@jorgeorpinel jorgeorpinel self-assigned this May 28, 2022
Comment on lines 100 to 105
"slug": "ml-experiment-tracking",
"label": "ML Experiment Tracking"
},
"experiment-tracking",
"model-registry",
"data-registry"
{
"slug": "ml-model-registry",
"label": "ML Model Registry"
Copy link
Contributor Author

@jorgeorpinel jorgeorpinel May 28, 2022

Choose a reason for hiding this comment

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

Hi @rogermparent qq: where do we add words for default capitalization from sidebar slugs? I'd like to make ml->"ML" to reduce these structures. Thanks

p.s. also "ML Frameworks" in the DVCLive docs

Copy link
Contributor

@julieg18 julieg18 May 30, 2022

Choose a reason for hiding this comment

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

Currently the code is written in a way that only dvc can be uppercased. But if we want to be able uppercase more than just DVC, it should be doable with a little code refactoring! I'll open a issue for it. (#3601)

Copy link
Contributor

Choose a reason for hiding this comment

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

#3602 has been merged and ml should become ML automatically :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Awesome, thanks Julie! Should this be a feature of the docs engine in general? CML and MLEM for example could use this 🙂

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes! Already opened a pr for adding this feature to our doc engine theme (See gatsby-theme-iterative#22)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dvclive - > "DVCLive" ? Or is that too much

Copy link
Member

Choose a reason for hiding this comment

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

a bit too much, Jorge (how many places do we have)?

Copy link
Contributor Author

@jorgeorpinel jorgeorpinel Jun 1, 2022

Choose a reason for hiding this comment

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

A few, same as the other ones. But yes never mind, it complicates the logic.

@gatsby-cloud
Copy link

gatsby-cloud bot commented May 29, 2022

Gatsby Cloud Build Report

dvc.org

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 2m

Performance

Lighthouse report

Metric Score
Performance 🔶 62
Accessibility 💚 98
Best Practices 🔶 83
SEO 💚 93

🔗 View full report

Comment on lines 508 to 530
{
"label": "CML",
"slug": "cml",
"source": "cml/index.md",
"label": "CML",
"icon": "cml",
"children": [
Copy link
Contributor Author

@jorgeorpinel jorgeorpinel May 29, 2022

Choose a reason for hiding this comment

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

cml -> "CML" would also be good to auto-capitalize. Plus how is this external link implemented now? The nav still has a substructure for CML (that is currently ignored). Seems like it's hard-coded somewhere? We can remove that 🙂 cc @rogermparent (lmk where and I will, thanks).

p.s. also "API" (used in DVCLive docs)

Copy link
Contributor

Choose a reason for hiding this comment

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

If I'm understanding correctly, we have a line in redirects.json that converts the /doc/cml to https://cml.dev/doc.

Comment on lines -526 to 502
"label": "CML",
"slug": "cml",
Copy link
Contributor Author

@jorgeorpinel jorgeorpinel Jun 1, 2022

Choose a reason for hiding this comment

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

For some reason removing the "CML" label hides the nav entry @julieg18 . All other auto-upper-case seem to work fine. Probably related with the redirect...

image

Copy link
Contributor

@julieg18 julieg18 Jun 1, 2022

Choose a reason for hiding this comment

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

Links with type: "external" actually need a label but no slug :)

  {
    "label": "CML",
    "icon": "cml",
    "url": "https://cml.dev/doc",
    "type": "external"
  },

And since our code only uppercases certain words when we are converting slugs to labels, the label needs to be uppercase on its own.

Copy link
Contributor Author

@jorgeorpinel jorgeorpinel Jun 1, 2022

Choose a reason for hiding this comment

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

Good to know, thanks (fixed in a74facb).

We should def. document the doc engine features and usage at some point 😬 rel. iterative/gatsby-theme-iterative#17

@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Jun 1, 2022

Anyway, this was a clean-up / proof of concept / test. Way to huge... I need to split it later (not a priority). Thanks though!

jorgeorpinel added a commit that referenced this pull request Jun 1, 2022
@jorgeorpinel jorgeorpinel added the A: docs Area: user documentation (gatsby-theme-iterative) label Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: docs Area: user documentation (gatsby-theme-iterative)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants