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

Adding deprecated warning icon/message in graphQL docs #2749

Merged
merged 6 commits into from
Oct 27, 2020

Conversation

antoine29
Copy link
Contributor

This PR is adding a warning icon to every deprecated field shown in the GraphQL schema explorer, also the deprecating reason is being shown in the icon tooltip. It closes #1831
deprecatedWarn

@nijikokun
Copy link
Contributor

Nice work 🎉

Is it possible to use the exclamation triangle instead of a circle to better signify more of a warning to the end user?

@develohpanda
Copy link
Contributor

develohpanda commented Oct 20, 2020

There's a warning icon in insomnia-components you might be able to use for it, will be consistent with other warnings in the code (such as the one you see in the GraphQL body in the screenshot above)

https://storybook.insomnia.rest/?path=/story/iconography-core-icons--reference

Copy link
Contributor

@develohpanda develohpanda left a comment

Choose a reason for hiding this comment

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

Nice work, thanks for the PR!

Comment on lines 81 to 82
const deprecatedFieldDescription = field =>
`The field ${field.name} is deprecated. ${field.deprecationReason}`;
Copy link
Contributor

@develohpanda develohpanda Oct 20, 2020

Choose a reason for hiding this comment

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

  1. Please could you move the creation of this function out of the render flow, so that the function is not recreated on every render?
  2. Please add speech marks around the field name (like The field "currentUser" is deprecated...) to make it a little clearer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey guys. Thank you for the feedback :D

  • I replaced the circle warning icon with the one from insomnia-components.
  • The deprecationDescription function was moved out from the render function.
  • Quote marks were added around the deprecated field name.
    fixedDeprecatedWarn

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice one. Do you know why the spacing looks a little off with the icon? I wonder if there is a rogue style being applied, adding padding/margin or something 🤔

Copy link
Contributor

@develohpanda develohpanda left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@nijikokun nijikokun left a comment

Choose a reason for hiding this comment

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

Nice work 🎉

Copy link
Contributor

@DMarby DMarby left a comment

Choose a reason for hiding this comment

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

Looks great, just tried it out against one of our APIs 👍

Thank you for taking the time to implement this!

@develohpanda develohpanda merged commit a511ea5 into Kong:develop Oct 27, 2020
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.

Show deprecation warning and message in GraphQL schema explorer
4 participants