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

THREESCALE-1789: Add "Managed by operator" banner (UI) #3876

Merged

Conversation

jlledom
Copy link
Contributor

@jlledom jlledom commented Sep 2, 2024

What this PR does / why we need it:

This replaces #3865. That got closed after merging #3857

The operator can manage porta resources through API. When a resource is managed by operator, we must inform the user about that.

#3857 implements the backend support (annotations). This PR implements the frontend, for now, just a banner in the relevant screens:

  • http://provider-admin.3scale.localhost:3000/buyers/accounts/4
  • http://provider-admin.3scale.localhost:3000/buyers/accounts/4/edit
  • http://provider-admin.3scale.localhost:3000/apiconfig/services/2
  • http://provider-admin.3scale.localhost:3000/apiconfig/services/2/edit
  • http://provider-admin.3scale.localhost:3000/p/admin/backend_apis/2
  • http://provider-admin.3scale.localhost:3000/p/admin/backend_apis/2/edit

Examples:

image

image

Which issue(s) this PR fixes

https://issues.redhat.com/browse/THREESCALE-1786

Verification steps

  1. Marks a resource as managed by operator. Check THREESCALE-1786: Managed by operator #3857 to know how
  2. Visit the resource #show or #edit screens
  3. You should see the banner

@jlledom jlledom self-assigned this Sep 2, 2024
@josemigallas
Copy link
Contributor

josemigallas commented Sep 2, 2024

It looks good like that, but I would rather follow Patternfly's design guidelines regarding inline alerts: https://www.patternfly.org/components/alert/design-guidelines#plain-inline-alerts:

@jlledom
Copy link
Contributor Author

jlledom commented Sep 3, 2024

It looks good like that, but I would rather follow Patternfly's design guidelines regarding inline alerts: https://www.patternfly.org/components/alert/design-guidelines#plain-inline-alerts:

I made some changes. Now it looks like this:

image

Now it uses a plan inline alert but there's no spacing between the title and the alert. I've been trying to fix it but I'm spending too much time and I think its better for somebody else with more experience to do it... @josemigallas

@jlledom
Copy link
Contributor Author

jlledom commented Sep 4, 2024

@josemigallas I made some changes, I'd say it looks better now:

image

image

@akostadinov
Copy link
Contributor

Maybe make orange or something? Looks good. Just I'm not sure anybody would notice blue stuff :) Blue means all is fine. idk. Just saying.

@jlledom
Copy link
Contributor Author

jlledom commented Sep 4, 2024

Maybe make orange or something? Looks good. Just I'm not sure anybody would notice blue stuff :) Blue means all is fine. idk. Just saying.

It would look like this:

image

WDYT @josemigallas

We can also have it as info for the #show action and warning for the #edit action

@akostadinov
Copy link
Contributor

Better to me. But just an opinion, do as you want.

@josemigallas
Copy link
Contributor

With the description it now looks weird 😅. Maybe we don't use .pf-m-plain when there is a description? Or never.

We can also have it as info for the #show action and warning for the #edit action

On the other hand, I agree with this ☝🏻

@jlledom
Copy link
Contributor Author

jlledom commented Sep 10, 2024

After the last changes @josemigallas @akostadinov

image

image

@lvillen
Copy link
Contributor

lvillen commented Sep 16, 2024

I think we could use the inline alert also at the #show action, which would make code climate happy, but beside that LGTM.

@jlledom
Copy link
Contributor Author

jlledom commented Sep 16, 2024

I think we could use the inline alert also at the #show action, which would make code climate happy, but beside that LGTM.

I added the plain style to the #show action as suggested by @josemigallas so I think we should keep it. However, I refactored the code to (hopefully) remove the warning from codeclimate: c5c1999

EDIT: Still a codeclimate error but that one was there before.

Copy link
Contributor

@lvillen lvillen left a comment

Choose a reason for hiding this comment

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

🚀

@jlledom jlledom force-pushed the THREESCALE-1789-rebased-managed-operator-ui branch from a24b25b to b40c707 Compare September 16, 2024 15:24
@jlledom jlledom merged commit 703614c into 3scale:master Sep 16, 2024
16 of 21 checks passed
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.

4 participants