Navigation Menu

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

Adds sorting to GQL Documentation Browser #2732

Merged
merged 4 commits into from Nov 19, 2020

Conversation

tomprats
Copy link
Contributor

@tomprats tomprats commented Oct 16, 2020

Really simple addition to sort list of graphql queries. Really helps in apps with a large number. Right now they're shown in a seemingly random order. Still might be better to have a full search (since ctrl-f doesn't seem to work), but this is a good start either way.

My original change was just:

+     {Object.keys(fields).map(key => {
-     {Object.keys(fields).sort().map(key => {

But eslint didn't like that. It's auto-fix did something like:

{Object.keys(fields)
  .sort()
  .map(key => {
    ...

Which indented most of the file a whole lot, so I wasn't sure if it was worth the large change log or if a variable was the better option. Let me know what you'd prefer and I can update it!

@develohpanda develohpanda changed the title Adds sorting to Documentation Browser - Closes #2720 Adds sorting to GQL Documentation Browser Oct 28, 2020
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.

Thanks @tomprats! This looks good to me.

@tomprats
Copy link
Contributor Author

Thanks for checking it out! I used the Github buttons to incorporate your change and get it up to date with develop. Let me know if there's anything else

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 good, thank you for implementing this @tomprats!

After talking to @nijikokun, I have unlinked the #2720 issue from this PR so that it will remain open after this is merged, as we'd still like to have search in the future as well.

@tomprats
Copy link
Contributor Author

@DMarby Any next steps I can do to get this merged? Would really love to have this functionality!

@develohpanda
Copy link
Contributor

Hi @tomprats, we've been pushing for an already large release and have halted merging open source PRs until then. Once the next release is out, we'll get through the remaining PRs and potentially do a patch release after. 😊

@tomprats
Copy link
Contributor Author

Sounds good! I appreciate your time

@develohpanda
Copy link
Contributor

develohpanda commented Nov 19, 2020

Netlify failure can be ignored, we need to investigate why it's occasionally failing to find rimraf, but your PR didn't break anything there. 😄

@develohpanda develohpanda merged commit b1259dd into Kong:develop Nov 19, 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.

None yet

3 participants