QwikDev / qwik

Instant-loading web apps, without effort
https://qwik.dev
MIT License
20.71k stars 1.29k forks source link

[✨]Adding visual clue in the docs TOC #6580

Open Kampouse opened 3 months ago

Kampouse commented 3 months ago

Is your feature request related to a problem?

no

Describe the solution you'd like

currently all the navigation links are in one color "Blue"

Describe alternatives you've considered

changing the font colour when you change section in the docs would be great the Nextjs docs as this current implementation image

Additional context

thanks you i am considering trying to do it! Is it worth the efforts?

thejackshelton commented 3 months ago

Sounds great to me 👍 . We do something similar in the Qwik UI Table of contents https://qwikui.com/docs/headless/introduction/

Although it's not a primary / active color it's bold. I think it's worth the effort, makes it more clear where you actually are on the page.

shairez commented 3 months ago

@Kampouse thanks! want to take a stab at it?

Kampouse commented 3 months ago

Yes!

thejackshelton commented 2 months ago

@Kampouse how's this going?

Kampouse commented 2 months ago

@thejackshelton i had some qwik issue image should look into what they were

thejackshelton commented 2 months ago

@thejackshelton i had some qwik issue image should look into what they were

Is there a link where I can look through your changes? Will check out the problem 👍

Kampouse commented 2 months ago

@thejackshelton i always ended up with this annoyance image

Kampouse commented 2 months ago

i will update my repository ti lastest to see if change anything

maiieul commented 2 months ago

@kampouse you might want to check out the toc component we have in Qwik UI for an accessible component (https://github.com/qwikifiers/qwik-ui/blob/main/apps/website/src/components/toc/toc.tsx). Although we to resort to a few hacks to make it more accessible with uls... So it might be better to wait a bit for it be more robust.

Until then you could use our previous implementation (https://github.com/qwikifiers/qwik-ui/blob/6626767f3d798a93e5d1a87fc99c9af98f0cd326/apps/website/src/components/toc/toc.tsx), which should make it easy to switch to our current one later. You can still use the useOnDocument that we use on the current implementation.

Feel free to DM me on Discord if you want to sync!

Kampouse commented 2 months ago

that exactly what i was using

TheMcnafaha commented 2 months ago

Calling it "hacks" is a bit of an exaggeration. It's all standard JS patterns that are also used in other repos like shadcn. I would also strongly advise against basing your work on the previous TOC for two reasons: (1) we have moved into a recursive pattern and (2) no longer use visible tasks. Both of these changes significantly alter the behavior of the component, meaning that a solution that works in the old TOC might not work in the current one. Plus, you will have nasty merge conflicts.

I contributed a lot to the refactor of the current TOC, so feel free to ask me any questions about it. My gut feeling is that you should look at the anchor component, especifically at the conditional class.

Kampouse commented 2 months ago

Hi i had i a bit more success with useWindow followed the https://github.com/qwikifiers/qwik-ui/blob/main/apps/website/src/components/toc/toc.tsx?rgh-link-date=2024-07-15T15%3A51%3A56Z and
image while it worked for some odd reason when removing my text "am active" the wholing returned to the qwik env issue

Kampouse commented 2 months ago

i open a pr related to this here https://github.com/QwikDev/qwik/pull/6689

TheMcnafaha commented 2 months ago

is this the desired result? image

Kampouse commented 2 months ago

yes this the desired result

TheMcnafaha commented 2 months ago

Then you actually have a good blueprint to use.

The big picture is that you need to implement the active item code from the qwikui TOC (look at the useActiveItem function) and then implement this anchor component:

const Anchor = component$<AnchorProps>(({ node, activeItem }) => {
  const isActive = node.id === activeItem;
  return (
    <a
      href={`#${node.id}`}
      onClick$={[
        $(() => {
          const element = document.getElementById(node.id);
          if (element) {
            const navbarHeight = 90;
            const position =
              element.getBoundingClientRect().top + window.scrollY - navbarHeight;
            window.scrollTo({ top: position, behavior: 'auto' });
          }
        }),
      ]}
      class={cn(
        node.level > 2 && 'ml-2',
        'inline-block no-underline transition-colors hover:text-foreground',
        isActive ? 'font-medium text-foreground text-red-400' : 'text-muted-foreground',
      )}
    >
      {node.text}
    </a>
  );
});

This should work as it doens't rely on recursion.