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

Add clients page #30

Merged
merged 6 commits into from
Mar 28, 2021
Merged

Add clients page #30

merged 6 commits into from
Mar 28, 2021

Conversation

CasperWA
Copy link
Member

The clients page showcases the various available clients to query OPTIMADE databases.

There are currently 3 different clients:

  • Swagger/OpenAPI UI (petstore.swagger.io)
  • MaterialsCloud Tool (materialscloud.org/optimadeclient)
  • optimade.science

To access this new page, a new menu item "Try It!" has been added.

I have requested reviews from the "management", providers I have explicitly mentioned, and client creators, and would especially like all entities who are mentioned on the new page to give their consent.

The clients page showcases the various available clients to query
OPTIMADE databases.
There are currently 3 different clients:
- Swagger/OpenAPI UI (petstore.swagger.io)
- MaterialsCloud Tool (materialscloud.org/optimadeclient)
- optimade.science

To access this new page, a new menu item "Try It!" has been added.
@CasperWA CasperWA added the enhancement New feature or request label Nov 23, 2020
Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

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

👍

@CasperWA
Copy link
Member Author

@blokhin is this page okay with you? Since it mentions you by name and points to your website, I want to make sure you're okay with it and/or if the text should be changed slightly.

@ml-evs Could I perhaps ask you to test this locally as well, like you did #29? Specifically if the menu does not look weird now with the extra entry.

Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

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

The menu is now a bit wide (wraps onto two lines when given < half the screen width at 1080p), but no big deal. Could potentially remove "Contributors" as a top-level link and put it under the About section and/or move forums to a link under Documentation?

I like the Try It page though (modulo minor fixes below), perhaps the menu button could be a brighter colour (OPTIMADE blue/red?) for emphasis? I guess a redesign is probably on the horizon anyway...

@blokhin
Copy link
Member

blokhin commented Nov 24, 2020

Please, could you delete the optimade.science for now? The current version is malfunctioning, and we are about to roll out the new major update soon (completely rewritten). I will PR it here as soon as it is ready.

CasperWA and others added 2 commits November 24, 2020 16:48
Fix link targets

Co-authored-by: Matthew Evans <7916000+ml-evs@users.noreply.github.com>
In accordance with the wish of @blokhin.
@CasperWA
Copy link
Member Author

The menu is now a bit wide (wraps onto two lines when given < half the screen width at 1080p), but no big deal. Could potentially remove "Contributors" as a top-level link and put it under the About section and/or move forums to a link under Documentation?

I can find pros and cons for keeping/removing them:
The Contributors list is quite a long one, not really fitting for a landing page, but it is indeed related to who the consortium is.
For the forum to be used, it needs a special place, hence a menu item makes sense, however, it again indeed goes under the "umbrella" of relating to documentation - so it would make sense to simply highlight on the documentation page. But I would need some input on this idea from @gmrigna, @shyamd and @mkhorton (who I believe set up this forum?).

(...) perhaps the menu button could be a brighter colour (OPTIMADE blue/red?) for emphasis? I guess a redesign is probably on the horizon anyway...

I was thinking the exact same thing! To catch the eye, but looking at it I would have to change a lot of the CSS and HTML design it seems. Not worth it for now 😅
And I don't know that a redesign in on the horizon as we already just did one...

Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

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

Fair enough, will just approve and let others decide.

I've just added b450a5d which highlights the Try It button in blue, feel free to revert before merging.

@gmrigna
Copy link
Contributor

gmrigna commented Nov 27, 2020

I will leave to @shyamd and @mkhorton to comment about the "Forum" button. I have no particular idea about where to place it...

@gmrigna
Copy link
Contributor

gmrigna commented Nov 27, 2020

The blue button looks good to me...

@ml-evs
Copy link
Member

ml-evs commented Mar 26, 2021

@CasperWA could you revert this commit? optimade.science is back again : )

I've just done this, so we should be good to go!

@ml-evs
Copy link
Member

ml-evs commented Mar 26, 2021

@CasperWA could you check that the changes (URLs etc) you made initially are still valid, then merge when you are happy?

@blokhin
Copy link
Member

blokhin commented Mar 26, 2021

I see the command {:target=_blank} is not rendered properly.

@ml-evs
Copy link
Member

ml-evs commented Mar 28, 2021

I see the command {:target=_blank} is not rendered properly.

That's fine, it works in the rendered HTML, we don't have to worry about the markdown here

@ml-evs
Copy link
Member

ml-evs commented Mar 28, 2021

I've tested all the client links locally, so I'm going to merge this so as not to block #31.

@ml-evs ml-evs merged commit f05c3f9 into Materials-Consortia:master Mar 28, 2021
@gmrigna
Copy link
Contributor

gmrigna commented Mar 29, 2021

In the "Swagger" section, the links to the Materials Project and the Materials Cloud lead to "Failed to load API definition."
The problem may be related to cross-origin (CORS) issue ("Access-Control-Allow-Origin").

@CasperWA CasperWA deleted the add_clients branch March 29, 2021 08:42
@CasperWA
Copy link
Member Author

CasperWA commented Mar 29, 2021

In the "Swagger" section, the links to the Materials Project and the Materials Cloud lead to "Failed to load API definition."
The problem may be related to cross-origin (CORS) issue ("Access-Control-Allow-Origin").

For Materials Cloud, I think this is related to the issue that one cannot reach the generated OpenAPI JSON for the implementations. See also Materials-Consortia/optimade-python-tools#520, which I think is the cause of the endpoint not being reachable.

Also, this was the reason for not yet merging it. But I guess it's fine. We should probably remove those links until it's fixed though!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants