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

⚛️ Finish React implementation of Plans index #3381

Merged
merged 12 commits into from
May 26, 2023

Conversation

josemigallas
Copy link
Contributor

@josemigallas josemigallas commented May 24, 2023

THREESCALE-9630: Unify plans components into a whole React page

Now all 3 plans pages are fully implemented with Patternfly React.

Before

Screenshot 2023-05-25 at 15 09 01
Screenshot 2023-05-25 at 15 08 55
Screenshot 2023-05-25 at 15 08 50

After

Screenshot 2023-05-25 at 15 02 17
Screenshot 2023-05-25 at 15 02 32
Screenshot 2023-05-25 at 15 02 39

@josemigallas josemigallas requested a review from a team May 24, 2023 14:52
@josemigallas josemigallas self-assigned this May 24, 2023
@thomasmaas
Copy link
Member

thomasmaas commented May 24, 2023

It says

service plans plans

Base automatically changed from THREESCALE-9647_page_headers to master May 25, 2023 12:54
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.

Just a small consistency change, but nice approach for plans-handling!

@mayorova
Copy link
Contributor

There is a weirdly aligned warning if no default plan is selected.
Screenshot from 2023-05-26 11-45-13

@mayorova
Copy link
Contributor

I'm wondering if it would be nice to actually make a single component, e.g. PlansIndexPage, and handle the differences with different props, i.e. pass "title", "subtitle", "helperText" and "warning" (showNotice thingy) to the component, and this way make it generic.

But it's OK if not. It's already very much unified 👍

@josemigallas
Copy link
Contributor Author

I'm wondering if it would be nice to actually make a single component, e.g. PlansIndexPage, and handle the differences with different props, i.e. pass "title", "subtitle", "helperText" and "warning" (showNotice thingy) to the component, and this way make it generic.

It's really hard to say when to stop making generic things. I think, since these are 3 different models, they belong to 3 different realms and therefore should have different controllers, presenters, etc.

@josemigallas
Copy link
Contributor Author

Before:
Screenshot 2023-05-26 at 13 27 37

After:
Screenshot 2023-05-26 at 16 45 50

@josemigallas josemigallas merged commit 02dd430 into master May 26, 2023
15 of 21 checks passed
@josemigallas josemigallas deleted the THREESCALE-9630_plans_page branch May 26, 2023 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants