Added oauth section to app details page. #104
Conversation
Signed-off-by: Nicholas Bucher <[email protected]>
Signed-off-by: Nicholas Bucher <[email protected]>
…to check for isAdmin prop. Signed-off-by: Nicholas Bucher <[email protected]>
Signed-off-by: Nicholas Bucher <[email protected]>
Signed-off-by: Nicholas Bucher <[email protected]>
|
Issues linked to changelog: |
Signed-off-by: Nicholas Bucher <[email protected]>
| }); | ||
| } | ||
| }} | ||
| additionalContentTop={ |
There was a problem hiding this comment.
I'm a bit surprised this isn't more standardized. This isn't really a 1-off is it?
There was a problem hiding this comment.
The ViewCreatedItemModal is being used in the Authorization section and the API Keys section. I wanted to not have 2 components doing the same thing, so that's why I added this additionalContentTop in and made it generic.
| }) => { | ||
| di(useDeleteOAuthMutation); | ||
| const { trigger: deleteOAuth } = useDeleteOAuthMutation(appId); | ||
| const onConfirm = async (e?: FormEvent) => { |
There was a problem hiding this comment.
Similar to above, I wonder that this isn't a standardized call as well (preventDefault, action, close)
There was a problem hiding this comment.
This whole component and a few others should be a ConfirmDeleteModal. As for the form event function being standardized/abstracted, I don't think I'd be against it, but I'd rather revisit that later.
| return bgImageURL; | ||
| } | ||
| if (!!bannerImageURL) { | ||
| return bannerImageURL; |
There was a problem hiding this comment.
Does React react to a change in a file that is imported? I'd believe it, but it doesn't seem intuitive
There was a problem hiding this comment.
Sorry, I see this is actually importing a variable from that file. Still, it feels like importing it would break the normal tracking.
Again, totally believe it, but it feels very weird.
There was a problem hiding this comment.
I'm fairly confident that this would result in the imports (both the Banner, and the bannerImageURL - which definitely could be named better) not causing any additional re-renders after the first render. So it would be the same as if the deps array was: [bgImageURL].
However, to be clearer and to not run into any React weirdness, I'll update it to just use [bgImageURL].
Signed-off-by: Nicholas Bucher <[email protected]>
This PR:
isAdmincheck so that the newisAdminproperty in the/meendpoint is preferred (it also still works with the old method of checking theisAdminproperty in the decoded access token).VITE_APIS_IMAGE_URLproperty toVITE_BANNER_IMAGE_URL, and now uses that, if it exists, as a fallback. This is now how users can customize the banner images on the apps + subscriptions + teams + apis pages.Here is a video of adding + deleting oauth clients:
Screen.Recording.2024-09-24.at.2.39.08.PM.mov
BOT NOTES:
resolves https://github.com/solo-io/solo-projects/issues/6886
resolves https://github.com/solo-io/solo-projects/issues/6953