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

BaseBadge Component #16

Closed
wants to merge 10 commits into from
Closed

BaseBadge Component #16

wants to merge 10 commits into from

Conversation

bryantgillespie
Copy link
Contributor

No description provided.

@bryantgillespie bryantgillespie marked this pull request as ready for review July 10, 2023 18:43
Comment on lines +56 to +57
// router-link is not displaying correctly in Histoire
return 'a';
Copy link
Member

Choose a reason for hiding this comment

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

But this should be a router-link? In that case, lets fix the problem in Histoire rather than in the website codebase itself 👍🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you give me some guidance on how you would you fix the problem in Histoire?

Copy link
Member

Choose a reason for hiding this comment

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

Looks like the problem wasn't histoire in the first place! You were trying to set an href on a router-link, but router-link uses to instead of href for local pages 👍🏻 For this badge, be aware that you need to use <a> for links to pages outside of the website, and <router-link> for pages within

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh - it was me 😆
Too used to using NuxtLink instead of router-link.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's the source for it
https://github.com/nuxt/nuxt/blob/main/packages/nuxt/src/app/components/nuxt-link.ts
It has some niceties built-in like prefetching and handling external links.

Is there an easy way to just use it inside of a Nuxt project vs a Vue project (docs)?

Copy link
Member

Choose a reason for hiding this comment

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

Is there an easy way to just use it inside of a Nuxt project vs a Vue project (docs)?

No idea, though I have my doubts it'll just work outside of the Nuxt context (as it's so dependent on the router and some other magic). It does raise the question though: should we rework the docs to Nuxt as well?

Copy link
Member

Choose a reason for hiding this comment

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

You mean adding Nuxt to context of Histoire?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does raise the question though: should we rework the docs to Nuxt as well?

I think so. Eventually...

I like VitePress and it's super easy to get started with but there are lots of little specific quirks that I've struggled to sort out.
But Nuxt has it's own quirks too.

To me it makes sense that it's all unified though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean adding Nuxt to context of Histoire?

No not really. I was just meaning if there was an easy way to say in this components package ..

hey if this is a nuxt project use nuxt-link if not use router-link

Copy link
Member

Choose a reason for hiding this comment

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

No not really. I was just meaning if there was an easy way to say in this components package ..

hey if this is a nuxt project use nuxt-link if not use router-link

For later: Something like this sounds like a good idea in order to be able to unify the components. For Nuxt we could maybe inject nuxt-link in the module setup file.

@paescuj
Copy link
Member

paescuj commented Jul 18, 2023

Closing in favor of #22.

@paescuj paescuj closed this Jul 18, 2023
@bryantgillespie bryantgillespie deleted the comp/base-badge branch March 27, 2024 14:37
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