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

API Explorer extension #7

Open
djmattyg007 opened this issue Jan 7, 2021 · 20 comments
Open

API Explorer extension #7

djmattyg007 opened this issue Jan 7, 2021 · 20 comments

Comments

@djmattyg007
Copy link
Contributor

As I mentioned on mopidy/mopidy#1958, I've taken the time to bring the API Explorer extension up to date. You can see my fork here:

https://github.com/djmattyg007/mopidy-api-explorer

I've submitted a pull request to the upstream repository here:

#6

However I noted on this PR that @kingosticks asked for this extension to be migrated to the Mopidy organisation:

#5

Is there still an inclination to get this extension into the Mopidy organisation?

@kingosticks
Copy link
Member

I am really happy about this, this is very useful. Thank you!

Is there still an inclination to get this extension into the Mopidy organisation?

I think I only did that because @dz0ny was no longer maintaining it. I don't think it matters much where it lives, especially if you have an interest in maintaining it (hopefully very little work)?

The remaining issue is transferring ownership of the project's PyPi account. Without that, we can't release under this name. But that is not a big deal, we have waited long enough now. Do you fancy renaming the project?

@kingosticks
Copy link
Member

I don't think it matters much where it lives, especially if you have an interest in maintaining it (hopefully very little work)?

But I think we would be happy for it to live in the Mopidy Org if you prefer. After all, it is kind of a "meta-extension" in the way it's primary use is to aid with developing for Mopidy, rather than simply using Mopidy.

@djmattyg007
Copy link
Contributor Author

I don't think it matters much where it lives, especially if you have an interest in maintaining it (hopefully very little work)?

I certainly hope the ongoing maintenance effort is minimal. Mopidy seems very stable at this point, so unless there were changes in Mopidy's core I can't see a reason for much to change soon. The only thing I'd change is updating to a non-beta version of Bootstrap 5 once it's released.

The remaining issue is transferring ownership of the project's PyPi account. Without that, we can't release under this name. But that is not a big deal, we have waited long enough now. Do you fancy renaming the project?

But I think we would be happy for it to live in the Mopidy Org if you prefer. After all, it is kind of a "meta-extension" in the way it's primary use is to aid with developing for Mopidy, rather than simply using Mopidy.

I wouldn't mind renaming the project. Did you have any ideas, or should we use something simplistic like "API-Explorer-NG" or "API-Explorer2"? I agree though that it would be better for the extension to live within the Mopidy Org.

I tweeted the original developer to see if we can get a response:

https://twitter.com/djmattyg007/status/1347310251066892290

@kingosticks
Copy link
Member

Or "JSONRPC-Explorer"? "HTTP-API-Explorer"? Yours are fine too. You did the work, I'm happy with your choice.

We did discuss recently about having something like what you get with OpenAPI/Swagger and then we'd call it "API-Playground". But that's way outside the scope of this issue.

@djmattyg007
Copy link
Contributor Author

I like "JSONRPC-Explorer", since that's most accurate. If another API was implemented, it would require a totally new implementation to handle. Do you think it's worth waiting to see if @dz0ny gets back to us on maintainership, or would you just like to go ahead with publishing a renamed project to the Mopidy Org?

@jodal
Copy link
Member

jodal commented Jan 8, 2021

I'd be very happy to see Mopidy-API-Explorer moved to the GitHub org with @djmattyg007 as the maintainer :-)

I just sent an email to @dz0ny requesting that he transfer the GitHub repo and PyPI project to us. Let's give him a few days before we make any forks.

@jodal jodal transferred this issue from mopidy/mopidy Jan 8, 2021
@jodal
Copy link
Member

jodal commented Jan 8, 2021

Janez responded swiftly, and the repo is now moved to the Mopidy organization, and this issue has even been moved into the repo. I'll get CI and release workflows up and running here sometime this weekend.

Feel free to start merging any improvements!

@djmattyg007
Copy link
Contributor Author

I don't appear to have maintainer controls for the repository. Was this still your intention?

@jodal
Copy link
Member

jodal commented Jan 9, 2021

I would still very much like you to take on the maintainership of Mopidy-API-Explorer. I added you to the Mopidy org now, so you have commit access to all repos. Welcome :-)

@djmattyg007
Copy link
Contributor Author

Well you've just made my day :)

@kingosticks
Copy link
Member

Awesome. Welcome aboard!

@jodal
Copy link
Member

jodal commented Jan 9, 2021

I've merged #6 and brushed up the project setup a bit. 🎉

I think this looks quite OK as is, but I'm leaving it up to @djmattyg007 to roll a release when he feels this is ready. I've set up the release workflow, so the process documented at https://docs.mopidy.com/en/latest/releasing/#releasing-extensions should work for this extension too.

After using the extension a bit, I have three suggestions, which you're free to ignore or postpone to a later release:

  • The blue navbar has low contrast. Consider changing the text color to white?
  • The page is very long. Consider splitting into different pages for intro/endpoints/further docs, methods, and events?
  • Could the use of promises in the JavaScript examples be replaced with await instead, ref https://github.com/mopidy/mopidy.js/#asyncawait?

@kingosticks
Copy link
Member

I never looked at the await version before, that is a lot cleaner. Interesting!

@djmattyg007
Copy link
Contributor Author

The blue navbar has low contrast. Consider changing the text color to white?

Yeah, I definitely think that's worth updating. My initial thought was that a different background colour is more appropriate. I'll play around with it and find something that looks better.

The page is very long. Consider splitting into different pages for intro/endpoints/further docs, methods, and events?

I'd like to break this off into a separate issue, as I think there's a couple of things to consider before making changes on this front.

Could the use of promises in the JavaScript examples be replaced with await instead

One way to reduce the verbosity of the documentation would be to reduce the JS examples to just the method call, and to provide a full example with await and try/catch just once near the top of the page. The per-method examples would then show nothing but the actual method call. Thoughts?

@kingosticks
Copy link
Member

One way to reduce the verbosity of the documentation would be to reduce the JS examples to just the method call, and to provide a full example with await and try/catch just once near the top of the page. The per-method examples would then show nothing but the actual method call. Thoughts?

Or by default we could hide all the example code in Bootstrap accordions (or with collapse directly)?

@djmattyg007
Copy link
Contributor Author

Or by default we could hide all the example code in Bootstrap accordions (or with collapse directly)?

Yeah, I like that idea. I'll put something together.

@djmattyg007
Copy link
Contributor Author

Or by default we could hide all the example code in Bootstrap accordions (or with collapse directly)?

What do you think about a tabbed interface?

https://getbootstrap.com/docs/5.0/components/navs-tabs/#javascript-behavior

@djmattyg007
Copy link
Contributor Author

Actually, I have a bigger idea.

  • Move the method documentation above the curl/javascript examples
  • Remove the explicit documentation heading
  • Use some heuristics to render the documentation text as HTML as sensibly as possible
  • Put the curl/javascript examples into tabs
  • Allow setting a default tab in the navbar, so a user can easily switch all examples to their preferred access method automatically

Thoughts?

@kingosticks
Copy link
Member

Back in the day, when I was finding this useful for (presumably) musicbox-webclient stuff, I used to focus on some particular method and having everything in the same place for that method was useful. It sounds like that would be lost with the proposal. But maybe that would be OK with some links to go back and forth between the description and code. And maybe other people didn't use it the same way as me so don't attribute too much weight to this view.

@djmattyg007
Copy link
Contributor Author

I've created a mockup of what I was envisaging. Ignore the mismatched scrollspy nav on the left side, I created this mockup by manually hacking up the DOM inside the browser.

2021-01-12_10-52

My idea is inspired by the Algolia documentation:

https://www.algolia.com/doc/api-client/getting-started/instantiate-client-index/

The idea is that most of the time you're likely working with just one API client, so you would select that from the dropdown in the navbar. However, you can easily switch API clients on a per-method basis at any time.

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

No branches or pull requests

3 participants