Skip to content

Conversation

@Blargian
Copy link
Member

@Blargian Blargian commented Jan 30, 2025

Summary

Temporary fix to the horrible top level navigation.

I think we can save some more space as well by renaming menu items:

"Getting started" -> "Get Started"
"Managing Data" -> "Manage Data"
"SQL Reference" -> "Reference" (It's not only about SQL)

  • "KB articles" becomes "KB" when the screen becomes to small to fit everything.

Checklist

@gingerwizard
Copy link
Collaborator

@gjones @crisalbu please, im not sure this works with the menus soo spread out on wide screens. I think it needs to be more adaptive.

@crisalbu
Copy link

I'm onboard with renaming them. Reference sounds a bit generic, will it be clear to users what this is?

@gingerwizard
Copy link
Collaborator

@crisalbu
Copy link

crisalbu commented Feb 3, 2025

I can see the improvement, much better now.

As the screen becomes more narrow and the menu items are more crammed, perhaps we can have Knowledge Base become KB (only on smaller screens, not by default).

@Blargian
Copy link
Member Author

Blargian commented Feb 3, 2025

I can see the improvement, much better now.

As the screen becomes more narrow and the menu items are more crammed, perhaps we can have Knowledge Base become KB (only on smaller screens, not by default).

@crisalbu Added. Would you mind taking a look please: https://clickhouse-docs-nxj0u7mte-clickhouse.vercel.app/docs/
If you're happy we can merge. The menu has been an eye sore for too long 😨

@crisalbu
Copy link

crisalbu commented Feb 3, 2025

I was wondering, how bout we make the nav items left aligned, like the top navigation. I would even go as far as using the same menu components we have for the top navigation, to make it feel more consistent.

If Dale is ok with using Reference, instead of SQL reference, I am also onboard,
Also, similar to SQL reference, should Server admin just become Admin?

@Blargian
Copy link
Member Author

Blargian commented Feb 4, 2025

@gingerwizard @crisalbu thoughts please on left aligned nav items: https://clickhouse-docs-6w6a3er2n-clickhouse.vercel.app/docs/

Personally I think it's an improvement with left-aligned items. I've tried to style the docs menu to look similar (more similar than it was, at least) to the top menu but for some reason it was implemented as a completely separate component to the top menu. Reworking the secondary menu to use the same components as the top menu will take a fair amount of effort.

I'm mainly concerned with fixing the overflow issue at this stage. Let's look at reworking the secondary menu further at a later stage.

Copy link

@crisalbu crisalbu left a comment

Choose a reason for hiding this comment

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

The items are still spread across the width, see:

Screenshot 2025-02-04 at 23 34 37

We should also set a new (and bigger) breakpoint, given that:

  • the CTAs (Sign-in and Start trial) drop in between the 2 menus
  • the chevron drops on a new line
Screenshot 2025-02-04 at 23 33 31

@Blargian
Copy link
Member Author

Blargian commented Feb 6, 2025

@crisalbu sorry for the delay, we hit a bump in the road with vercel builds yesterday so couldn't share the build. Here are the latest changes: https://clickhouse-docs-ps0f1dhfr-clickhouse.vercel.app/docs/

Have added a few more breakpoints so that:

  • on larger screens the secondary menu items don't stretch out too much, as you observed.
  • on smaller screens we don't see the 'get started' button drop to a new line. Github stars get hidden in a small interval before we switch to mobile but then they display again. Additionally the button size shrinks slightly.
  • "Dark mode" changes to just "Dark" or "Light" in a small interval before mobile.

Does this work?

@crisalbu
Copy link

crisalbu commented Feb 6, 2025

So much better!

One last request, can we make these 2 menus right aligned?

Screenshot 2025-02-06 at 09 18 27

@Blargian
Copy link
Member Author

Blargian commented Feb 6, 2025

@crisalbu fixed (and some other small issues i noticed) - https://clickhouse-docs-cx95xobw9-clickhouse.vercel.app/

.click-button {
padding: 8px;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit:

.githubStars {
display: none;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

address

[data-theme="light"] .docsNavDropdownContainer {
background-color: white;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit:

Copy link
Collaborator

@gingerwizard gingerwizard left a comment

Choose a reason for hiding this comment

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

minor

@gingerwizard gingerwizard self-requested a review February 6, 2025 11:34
@gingerwizard gingerwizard merged commit 1d2461e into ClickHouse:main Feb 6, 2025
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants