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
Feat/environment view #1053
Feat/environment view #1053
Conversation
I'd go with a green icon instead of a black one on the menu. Can't really see it. Not sure how the other icons are being shown in dark mode. 🤔 Are all icons black? BTW, not sure if you've seen this comment: #863 (comment) Let's check those, and the remaining stuff after we do the current release. I like the general idea in the dialog, but I feel some additional styling might be added (for example, different fonts for the subtitle and the actual link). |
Okay yea I can add it to the places we mentioned before: |
After talking with the team, I'll be changing the icon to be added to the |
…iny-js into feat/environment-view
@doitadrian the idea was to have one single place with all API URLs. So users don't have to use terminal to find the main GraphQL API URL. I agree that we need to iterate on this visually, just to make real distinction between the 2 sections, but I would definitely keep this information all in one place. |
Agreed, I'm also for keeping åll of the info in one place. 👍 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check my comment.
BTW please also divide GraphQL API
row from the Headless CMS - Production
section title.
packages/app-admin/package.json
Outdated
@@ -10,6 +10,7 @@ | |||
"author": "Pavel Denisjuk", | |||
"license": "MIT", | |||
"dependencies": { | |||
"@apollo/react-hooks": "^3.1.5", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this for? o.O
@@ -0,0 +1,136 @@ | |||
import gql from "graphql-tag"; | |||
|
|||
const fields = ` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These queries are specific to app-headless-cms
, and are made in the app-admin
package.
This should not be here.
I suggest you leave this dialog here, but make it also pluggable, so that the additional Headless CMS URLs section can be added from that app, via a plugin.
I see you have the EnvironmentInfoDialog
component there. I would rename it to APIInformationDialog
, and add the Headless CMS section via a new app-plugin-admin-information-dialog
plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@EmilK15 this is the same thing you did for the WelcomeScreen
. Create a skeleton which uses plugins, but the plugins themselves must come from other apps. App dependencies must not be hardcoded inside the packages, meaning, app-admin
must NOT depend on any other app, because it's a skeleton, not an actual implementation. That's the whole point of plugins.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Pavel910 for additional explanation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great points,
So to recap the plan is for that the component within app-admin
will render a plugin that will be defined within app-headless-cms
which will contain the environment information content.
In doing this we won't need to make a query that was specific to app-headless-cms
from app-admin
.
Makes sense to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, let us know if you'll need any help with this.
Related Issue
Closes #863
Your solution
Added information icon next to the name of the environment.
Added EnvironmentInfoDialog component which will display and allow users to copy the URL of their api endpoints.
How Has This Been Tested?
Made sure that if you switch to different environments that the dialog also gets updated
Screenshots (if relevant):