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

refactor: [M3-7230] - Migrate NodeBalancer Node Mode Select TextField to Autocomplete #9754

Conversation

bnussman-akamai
Copy link
Member

@bnussman-akamai bnussman-akamai commented Oct 4, 2023

Description 📝

Major Changes 🔄

  • Clean up Remove button styles
  • Replace <TextField select>...</TextField> with an <Autocomplete />
  • Removes some unused testing attributes

Preview 📷

Before After
Screenshot 2023-10-04 at 12 28 13 PM Screenshot 2023-10-04 at 12 27 57 PM

How to test 🧪

  • Deploy a Linode with a private ipv4
  • Go create a NodeBalancer in the same region as that Linode
  • Add your Linode as a node and complete the NodeBalancer creation
  • Go to that NodeBalancer's config and test the Mode Select
    • Test styles 🎨
    • Test functionality 🎮

@bnussman-akamai bnussman-akamai requested a review from a team as a code owner October 4, 2023 16:31
@bnussman-akamai bnussman-akamai removed the request for review from a team October 4, 2023 16:31
@bnussman-akamai bnussman-akamai self-assigned this Oct 4, 2023
Copy link
Contributor

@hana-akamai hana-akamai left a comment

Choose a reason for hiding this comment

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

Looks like removing a Backend Node broke 🤔

Screen.Recording.2023-10-05.at.5.03.23.PM.mov

@bnussman-akamai
Copy link
Member Author

@hana-linode Thank you! Fixed in eb9667a

Copy link
Contributor

@hana-akamai hana-akamai left a comment

Choose a reason for hiding this comment

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

Confirmed that the Mode select still looks and works as expected and that remove still works 🎉

@bnussman-akamai bnussman-akamai added the Add'tl Approval Needed Waiting on another approval! label Oct 12, 2023
Copy link
Contributor

@jdamore-linode jdamore-linode left a comment

Choose a reason for hiding this comment

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

Thanks @bnussman-akamai! Confirmed that the autocomplete field works/looks as expected and that I can add/remove nodes. Also confirmed that I can actually save the config changes

@bnussman-akamai bnussman-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Oct 12, 2023
@bnussman-akamai bnussman-akamai merged commit addbf1c into linode:develop Oct 12, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants