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(dashboard): Tag input suggestions #6728

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

desiprisg
Copy link
Contributor

What changed? Why was the change needed?

  • Removed barrel files
  • Added the suggestions for tag inputs

Screenshots

2024-10-21.12-05-24.mp4
Expand for optional sections

Related enterprise PR

Special notes for your reviewer

Copy link

linear bot commented Oct 21, 2024

@desiprisg desiprisg force-pushed the nv-4504-integrate-tag-input-suggestions branch from a224399 to 0bea6e6 Compare October 21, 2024 09:10
Copy link

pkg-pr-new bot commented Oct 21, 2024

Open in Stackblitz

@novu/client

pnpm add https://pkg.pr.new/novuhq/novu/@novu/client@6728

@novu/framework

pnpm add https://pkg.pr.new/novuhq/novu/@novu/framework@6728

@novu/headless

pnpm add https://pkg.pr.new/novuhq/novu/@novu/headless@6728

@novu/nest

pnpm add https://pkg.pr.new/novuhq/novu/@novu/nest@6728

@novu/nextjs

pnpm add https://pkg.pr.new/novuhq/novu/@novu/nextjs@6728

@novu/js

pnpm add https://pkg.pr.new/novuhq/novu/@novu/js@6728

@novu/node

pnpm add https://pkg.pr.new/novuhq/novu/@novu/node@6728

@novu/notification-center

pnpm add https://pkg.pr.new/novuhq/novu/@novu/notification-center@6728

@novu/react

pnpm add https://pkg.pr.new/novuhq/novu/@novu/react@6728

@novu/providers

pnpm add https://pkg.pr.new/novuhq/novu/@novu/providers@6728

@novu/react-native

pnpm add https://pkg.pr.new/novuhq/novu/@novu/react-native@6728

@novu/shared

pnpm add https://pkg.pr.new/novuhq/novu/@novu/shared@6728

novu

pnpm add https://pkg.pr.new/novuhq/novu@6728

@novu/stateless

pnpm add https://pkg.pr.new/novuhq/novu/@novu/stateless@6728

commit: 9504e0a

Copy link

netlify bot commented Oct 21, 2024

Deploy Preview for novu-stg-vite-dashboard-poc ready!

Name Link
🔨 Latest commit 9504e0a
🔍 Latest deploy log https://app.netlify.com/sites/novu-stg-vite-dashboard-poc/deploys/671646b595a6e200088df361
😎 Deploy Preview https://deploy-preview-6728--novu-stg-vite-dashboard-poc.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 configuration.

@@ -54,6 +55,12 @@ export const CreateWorkflowButton = (props: CreateWorkflowButtonProps) => {
setIsOpen(false);
},
});
const { data: tagData } = useQuery<{ data: { name: string }[] }>({
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's factor this out to a dedicated useTags hook. We will need it in other screens as well.

Copy link
Contributor Author

@desiprisg desiprisg Oct 21, 2024

Choose a reason for hiding this comment

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

Done. I added a useTagsQuery here and I think that pattern will suit us better than filtering out what we pass / return. @LetItRock Let's discuss this and see what pattern will fit us best.

Copy link
Contributor

@LetItRock LetItRock Oct 21, 2024

Choose a reason for hiding this comment

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

@desiprisg I'm totally fine with following useXQuery, useXMutation pattern ;) let's just quickly update the hooks in the separate ticket

Copy link
Contributor

Choose a reason for hiding this comment

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

Folks, the one we have now is OK as well. So please align them separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also meant not passing onSuccess and onError funcs, and just passing through what @tanstack/react-query accepts.

apps/dashboard/src/components/primitives/tag-input.tsx Outdated Show resolved Hide resolved
apps/dashboard/src/components/primitives/tag-input.tsx Outdated Show resolved Hide resolved
@desiprisg desiprisg force-pushed the nv-4504-integrate-tag-input-suggestions branch 2 times, most recently from 8ca1212 to 536c705 Compare October 21, 2024 09:48
const { currentEnvironment } = useEnvironment();
const query = useQuery<{ data: { name: string }[] }>({
queryKey: [QueryKeys.fetchWorkflow, currentEnvironment?._id],
queryFn: async () => await getV2(`/environments/${currentEnvironment!._id}/tags`),
Copy link
Contributor

Choose a reason for hiding this comment

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

please move it to the API helper function, then we won't need to do data.data when using the query hook

Copy link
Contributor Author

@desiprisg desiprisg Oct 21, 2024

Choose a reason for hiding this comment

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

I don't think this is correct. If it's a useTagsQuery hook, then you should get back whatever was in the body, right? If data doesn't make sense, then that's a BE concern I guess.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

merged in order to get the import changes in. If you feel strongly about this, let's discuss privately.

@desiprisg desiprisg force-pushed the nv-4504-integrate-tag-input-suggestions branch from 536c705 to 9504e0a Compare October 21, 2024 12:18
@desiprisg desiprisg merged commit e8fd24e into next Oct 21, 2024
42 checks passed
@desiprisg desiprisg deleted the nv-4504-integrate-tag-input-suggestions branch October 21, 2024 13:56
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