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.

Sign up for GitHub

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

Jump to bottom

refactor: [M3-5188] - React Query for NodeBalancers #8964

Merged
merged 23 commits into from
Apr 7, 2023
Merged

refactor: [M3-5188] - React Query for NodeBalancers #8964

merged 23 commits into from
Apr 7, 2023

Conversation

bnussman-akamai
Copy link
Member

@bnussman-akamai bnussman-akamai commented Apr 4, 2023 •
edited
Loading

Description 📝

  • Modernizes and React Query-ifys most NodeBalancer features of Cloud Manager

Improvements ✨

  • Improves the UI of the NodeBalancer Create page
    • It now matches the Linode Create Page

Bug Fixes 🐛

  • Clicking on a port configuration from the NodeBalancers landing page now brings to the details page with the click config open

How to test 🧪

  • Test all NodeBalancer create, update, and delete actions

@bnussman-akamai bnussman-akamai added React Query Relating to the transition to use React Query Ready for Review labels Apr 4, 2023
@bnussman-akamai bnussman-akamai self-assigned this Apr 4, 2023
@bnussman-akamai
Copy link
Member Author

After both this PR and #8952 are in develop, I will make another PR that makes this abide by the new pattern.

jaalah-akamai
jaalah-akamai reviewed Apr 6, 2023
View reviewed changes
Copy link
Contributor

@jaalah-akamai jaalah-akamai 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 few minor changes, overall it's good and we'll refactor more later 👍

packages/manager/src/features/NodeBalancers/NodeBalancerDetail/NodeBalancerSettings.tsx Outdated Show resolved Hide resolved
packages/manager/src/features/NodeBalancers/NodeBalancerDetail/NodeBalancerSettings.tsx Show resolved Hide resolved
packages/manager/src/features/NodeBalancers/NodeBalancerDetail/NodeBalancerSettings.tsx Outdated Show resolved Hide resolved
hkhalil-akamai
hkhalil-akamai requested changes Apr 6, 2023
View reviewed changes
Copy link
Contributor

@hkhalil-akamai hkhalil-akamai left a comment

Choose a reason for hiding this comment

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

Left some minor feedback and one regression. Overall, changes look good.

packages/manager/src/containers/regions.container.tsx Outdated Show resolved Hide resolved
packages/manager/src/features/NodeBalancers/NodeBalancerDetail/NodeBalancerSettings.tsx Show resolved Hide resolved
packages/manager/src/features/Search/withStoreSearch.tsx Outdated Show resolved Hide resolved
hkhalil-akamai
hkhalil-akamai approved these changes Apr 6, 2023
View reviewed changes
Copy link
Contributor

@hkhalil-akamai hkhalil-akamai left a comment

Choose a reason for hiding this comment

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

LGTM!

@bnussman-akamai bnussman-akamai added the Approved Multiple approvals and ready to merge! label Apr 6, 2023
@bnussman-akamai bnussman-akamai merged commit 0fa799f into linode:develop Apr 7, 2023
This was referenced Apr 7, 2023
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaalah-akamai jaalah-akamai jaalah-akamai approved these changes

@hkhalil-akamai hkhalil-akamai hkhalil-akamai approved these changes

Assignees

@bnussman-akamai bnussman-akamai

Labels
Approved Multiple approvals and ready to merge! React Query Relating to the transition to use React Query
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bnussman-akamai @hkhalil-akamai @jaalah-akamai @bnussman