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

feat(navigation): allow passing QueryBuilder or QueryBuilderParams in fetchNavigation or <ContentNavigation> #1206

Merged
merged 7 commits into from Jun 3, 2022

Conversation

harlan-zw
Copy link
Contributor

@harlan-zw harlan-zw commented Jun 3, 2022

๐Ÿ”— Linked issue

โ“ Type of change

  • ๐Ÿ“– Documentation (updates to the documentation or readme)
  • ๐Ÿž Bug fix (a non-breaking change that fixes an issue)
  • ๐Ÿ‘Œ Enhancement (improving an existing functionality like performance)
  • โœจ New feature (a non-breaking change that adds functionality)
  • โš ๏ธ Breaking change (fix or feature that would cause existing functionality to change)

๐Ÿ“š Description

The ContentNavigation component is typed to only allow you to provide the query as QueryBuilderParams. The DX of this isn't great as everywhere else in the doc a QueryBuilder is used with queryContent.

This is especially annoying when you want to show the ContentNavigation with a simple path prefix, as the user has to then figure out regex.

The fetchContentNavigation already supports a QueryBuilder, so it's seems like this was the initial expectation of this prop anyway.

I've added an example to highlight the use case

๐Ÿ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@netlify
Copy link

netlify bot commented Jun 3, 2022

โœ… Deploy Preview for nuxt-content ready!

Name Link
๐Ÿ”จ Latest commit 267e4b7
๐Ÿ” Latest deploy log https://app.netlify.com/sites/nuxt-content/deploys/629a234ccdc87100098d6de6
๐Ÿ˜Ž Deploy Preview https://deploy-preview-1206--nuxt-content.netlify.app
๐Ÿ“ฑ Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Member

@Atinux Atinux left a comment

Choose a reason for hiding this comment

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

Beautiful โค๏ธ

docs/content/4.api/1.components/3.content-navigation.md Outdated Show resolved Hide resolved
docs/content/4.api/1.components/3.content-navigation.md Outdated Show resolved Hide resolved
Co-authored-by: Sรฉbastien Chopin <seb@nuxtjs.com>
@Tahul
Copy link
Contributor

Tahul commented Jun 3, 2022

I love it as well! :)

Thanks a lot @harlan-zw ๐Ÿ˜„

@Tahul
Copy link
Contributor

Tahul commented Jun 3, 2022

Hey @harlan-zw :)

After a brief discussion with @farnabaz ; I remembered we had plans on supporting the same type of query format as <ContentQuery /> has.

I will still merge this PR after applying some changes as we want to support both:

<script setup>
const queryBuilder = queryContent().where({ ... })
</script>

<template>
   <ContentNavigation :query="queryBuilder" />
   <!-- OR (upcoming) -->
   <ContentNavigation :where="{ ... }" />
</template>

@Tahul
Copy link
Contributor

Tahul commented Jun 3, 2022

Just added a test to cover your changes @harlan-zw, thank you again for this!

@Tahul Tahul changed the title feat(ContentNavigation): allow query prop to be QueryBuilder instance feat(navigation): allow passing QueryBuilder or QueryBuilderParams in fetchNavigation or <ContentNavigation> Jun 3, 2022
@Tahul Tahul merged commit afb791b into nuxt:main Jun 3, 2022
@harlan-zw
Copy link
Contributor Author

Hey @harlan-zw :)

After a brief discussion with @farnabaz ; I remembered we had plans on supporting the same type of query format as <ContentQuery /> has.

I will still merge this PR after applying some changes as we want to support both:

<script setup>
const queryBuilder = queryContent().where({ ... })
</script>

<template>
   <ContentNavigation :query="queryBuilder" />
   <!-- OR (upcoming) -->
   <ContentNavigation :where="{ ... }" />
</template>

This would definitely be the ideal case, having all components with the same prop API for querying would be a great DX improvement.

@farnabaz farnabaz mentioned this pull request Aug 12, 2022
farnabaz pushed a commit that referenced this pull request Sep 7, 2022
โ€ฆ `fetchNavigation` or `<ContentNavigation>` (#1206)

Co-authored-by: Sรฉbastien Chopin <seb@nuxtjs.com>
Co-authored-by: Yaรซl Guilloux <yael.guilloux@gmail.com>
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.

None yet

3 participants