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

[@kadena/graph-client] Added query information on each page #1394

Merged
merged 3 commits into from
Jan 2, 2024

Conversation

MRVDH
Copy link
Contributor

@MRVDH MRVDH commented Dec 27, 2023

chrome_1vc8fPoF9Y

Copy link

changeset-bot bot commented Dec 27, 2023

🦋 Changeset detected

Latest commit: 6081855

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@kadena/graph-client Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Dec 27, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

5 Ignored Deployments
Name Status Preview Comments Updated (UTC)
alpha-docs ⬜️ Ignored (Inspect) Visit Preview Jan 2, 2024 10:31am
docs-storybook ⬜️ Ignored (Inspect) Visit Preview Jan 2, 2024 10:31am
proof-of-us ⬜️ Ignored (Inspect) Visit Preview Jan 2, 2024 10:31am
react-ui ⬜️ Ignored (Inspect) Visit Preview Jan 2, 2024 10:31am
tools ⬜️ Ignored (Inspect) Visit Preview Jan 2, 2024 10:31am

{() => (
<DialogContent>
<p>Query</p>
{queries.map((query, index) => (
Copy link
Contributor

@nil-amrutlal nil-amrutlal Dec 28, 2023

Choose a reason for hiding this comment

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

If there's more than one query with variables, does it still make sense to represent them in the same <DialogContent>? My concern is only if the variables happen to have the same name, it can be confusing to determine which one belongs to which.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep you're right. I have redone it to make more sense. Let me know what you think!

chrome_1vc8fPoF9Y

@nil-amrutlal
Copy link
Contributor

Looks good 🙌🏽 just left a small comment. Also, maybe it would be nice to have it on the right side of the page? Just because I think its very close to the breadcrumbs. In any case, it is prob going to be changed again when we have a design so wouldn't bother too much with it
image

@MRVDH
Copy link
Contributor Author

MRVDH commented Jan 2, 2024

Looks good 🙌🏽 just left a small comment. Also, maybe it would be nice to have it on the right side of the page? Just because I think its very close to the breadcrumbs. In any case, it is prob going to be changed again when we have a design so wouldn't bother too much with it snip

Makes more sense indeed. Moved to the right!

chrome_BWVmzApT40

@MRVDH MRVDH merged commit e78e23b into main Jan 2, 2024
13 checks passed
@MRVDH MRVDH deleted the feat/graph-client/query-info-on-every-page branch January 2, 2024 10:46
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.

2 participants