refactor: [M3-6908] - Replace react-select in Profile #10780

carrillo-erik · 2024-08-13T18:02:27Z

Description 📝

This PR helps remove part of the react-select dependency for accessibility reasons and to consolidate our usage of third-party libraries. In particular, it replaces the EnhancedSelect with our more versatile MUI-based Autocomplete component within the Profile feature.

Changes 🔄

Introduce a new sxPopperComponent prop (optional) for the Autocomplete which allows custom CSS styling for the PopperComponent.
The PhoneVerification depended heavily on the react-select instance, and by removing it, the previous styling required to be redone. This was the reason for introducing the new sxPopperComponent prop.
Update TimezoneForm to TimeZoneForm.
Update default exports with named exports and corresponding lazy() imports.

Target release date 🗓️

09/03/2024

How to test 🧪

Verification steps

(How to verify changes)

Verify that the Profile feature works as expected.
Inspect the Profile pages for any visual regressions.

As an Author I have considered 🤔

Check all that apply

…ctor/M3-6908

cpathipa

@carrillo-erik The build failed, not able to load the module TimeZoneForm file name should be renamed to TimeZoneForm instead of TimezoneForm

github-actions · 2024-08-23T16:19:08Z

Coverage Report: ✅
Base Coverage: 86.64%
Current Coverage: 86.64%

mjac0bs

Thanks for the clean up with Autocompletes. I'm seeing a few things left to address here:

There's one more failure that is legit and a result of these changes: security-questions.spec.ts. It's still looking for the enhanced select.

I'm seeing a lot of console warnings about uncontrolled vs controlled when changing the values of the Autocomplete fields. I'd check to see how we're handling this in other uses of Autocomplete throughout the app.

Console Errors

Styling regressions on Phone Verification field (see comments)
Regression with disabled state on the LISH Authentication options (see comment)

mjac0bs · 2024-08-26T17:28:34Z

.../manager/src/features/Profile/AuthenticationSettings/PhoneVerification/PhoneVerification.tsx

@@ -166,22 +175,10 @@ export const PhoneVerification = ({
      );
  };

-  const customStyles = {


I noticed a regression with this field in dark mode; it's much more difficult to tell in light mode unless you fill the entire textfield with text and notice that it doesn't go all the way to the edge of the field. It looks like maybe the width of the container isn't correct anymore because the background doesn't fill the input area.

Prod This Branch

mjac0bs · 2024-08-26T17:31:59Z

.../manager/src/features/Profile/AuthenticationSettings/PhoneVerification/PhoneVerification.tsx

@@ -260,32 +258,37 @@ export const PhoneVerification = ({
                isPhoneInputFocused={isPhoneInputFocused}
              >
                <StyledSelect


In prod, we couldn't erase the flag, but now we can, and that leads to some slightly odd behavior where, if the flag is cleared, the country code is still there. Also, the placeholder "Select" gets cut off. Are we able to make the country non-clearable?

Light mode is this branch, dark mode is prod:

Screen.Recording.2024-08-26.at.10.15.08.AM.mov

mjac0bs · 2024-08-26T17:51:01Z

packages/manager/src/features/Profile/LishSettings/LishSettings.tsx

 export const LishSettings = () => {
  const theme = useTheme();
  const { data: profile, isLoading } = useProfile();
  const { mutateAsync: updateProfile } = useMutateProfile();
  const [submitting, setSubmitting] = React.useState<boolean>(false);
+  const [errors, setErrors] = React.useState<APIError[]>([]);
+  const [success, setSuccess] = React.useState<string>();
+  const thirdPartyEnabled = profile?.authentication_type !== 'password';


We've got a regression here where the thirdPartyEnabled isn't disabling an option anymore as expected. See screenshots. (I'm using my personal prod account with SSO authentication, not password auth.)

Prod This Branch

This regression is still happening on the current branch - I shouldn't be able to select this option.

@mjac0bs After the brief exchange we had, I went and set up SSO auth on my account and was able to verify that my latest changes resolved this issue.

carrillo-erik · 2024-08-27T17:52:44Z

@mjac0bs

There's one more failure that is legit and a result of these changes: security-questions.spec.ts. It's still looking for the enhanced select.

I've fixed the issue with the test looking for the old enhanced select component and is now passing locally.

I'm seeing a lot of console warnings about uncontrolled vs controlled when changing the values of the Autocomplete fields. I'd check to see how we're handling this in other uses of Autocomplete throughout the app.

These have been addressed with my latest changes. Thank you for inspecting the browser console and pointing these out.

cpathipa

Functionality LGTM! left few comments related to styling.

cpathipa · 2024-08-29T17:10:44Z

packages/manager/src/features/Profile/AuthenticationSettings/SecurityQuestions/Question.tsx

@@ -55,12 +54,18 @@ export const Question = (props: Props) => {
    );
  }
  return (
-    <Select
+    <Autocomplete


Can we make the UI consistent selected icon is missing for the selected option.

Example

@cpathipa I do see the point that you're making. However, after investigating, it appears that this is caused by the underlying usage of formik. For example, you'll notice the same discrepancy in the Image and Region select components in the Linode Create flow. This might be better suited for a separate ticket to identify which components need to be updated to use formik to maintain consistency.

cpathipa · 2024-08-29T17:12:44Z

packages/manager/src/features/Profile/AuthenticationSettings/SecurityQuestions/Question.tsx

@@ -55,12 +54,18 @@ export const Question = (props: Props) => {
    );
  }
  return (
-    <Select
+    <Autocomplete
+      onChange={(_, item) => {


Also, by default the the first option in the list is highlighted instead of selected option

@cpathipa Thanks for catching this. I've made the necessary changes to highlight the most recent selections rather than the first option.

cpathipa · 2024-08-29T17:19:29Z

.../manager/src/features/Profile/AuthenticationSettings/PhoneVerification/PhoneVerification.tsx

+                  onChange={(_, item: SelectPhoneVerificationOption) => {
+                    sendCodeForm.setFieldValue('iso_code', item.value);
+                  }}
+                  options={countries.map((country) => ({


Also, The border color for the portion of select field is not matching the rest of the field as highlighted in the screenshot.

This is difficult to see, but I'm still noticing this as well.

Prod This Branch

@cpathipa @mjac0bs This UI bug has finally been squashed! Feel free to also test the dark mode view.

Nice, thank you!

I was seeing one weird thing here, and it might have to do with the default Autocomplete behavior: the flag icon is now selected when the dropdown is open. In dark mode, this gives the appearance that the flag's background is white until a selection is made or the user moves the cursor.

Screen.Recording.2024-09-12.at.10.57.04.AM.mov

mjac0bs · 2024-09-03T16:42:18Z

@carrillo-erik Looks like there's a merge conflict that will need to be resolved. I'll take a final pass at this once this branch is up to date and Chandra's UX feedback is addressed. Thanks for addressing initial feedback.

…ctor/M3-6908

mjac0bs · 2024-09-06T20:20:52Z

packages/manager/src/features/Profile/LishSettings/LishSettings.tsx

 export const LishSettings = () => {
  const theme = useTheme();
  const { data: profile, isLoading } = useProfile();
  const { mutateAsync: updateProfile } = useMutateProfile();
  const [submitting, setSubmitting] = React.useState<boolean>(false);
+  const [errors, setErrors] = React.useState<APIError[]>([]);
+  const [success, setSuccess] = React.useState<string>();
+  const thirdPartyEnabled = profile?.authentication_type !== 'password';


This regression is still happening on the current branch - I shouldn't be able to select this option.

...er/src/features/Profile/AuthenticationSettings/PhoneVerification/PhoneVerification.styles.ts

packages/manager/cypress/e2e/core/account/security-questions.spec.ts

mjac0bs · 2024-09-06T20:30:08Z

.../manager/src/features/Profile/AuthenticationSettings/PhoneVerification/PhoneVerification.tsx

+                  onChange={(_, item: SelectPhoneVerificationOption) => {
+                    sendCodeForm.setFieldValue('iso_code', item.value);
+                  }}
+                  options={countries.map((country) => ({


This is difficult to see, but I'm still noticing this as well.

Prod This Branch

…ctor/M3-6908

mjac0bs

Noted one minor thing about dark mode county code selection, but otherwise it looks like feedback has been addressed here and relevant tests are passing. 👍🏼

refactor: [M3-6908] - Replace react-select in Profile

d012b8e

carrillo-erik self-assigned this Aug 13, 2024

carrillo-erik added 5 commits August 20, 2024 12:04

Update PhoneVerification styles

6b7baf6

Comment out styles

aee6afa

Updates to Autocomplete to allow custom styles for PopperComponent

6e4a51a

Reapply updates to CreateAPITokenDrawer

5b08296

Merge branch 'develop' of https://github.com/linode/manager into refa…

b6807a0

…ctor/M3-6908

carrillo-erik marked this pull request as ready for review August 21, 2024 15:50

carrillo-erik requested a review from a team as a code owner August 21, 2024 15:50

carrillo-erik requested review from bnussman-akamai, cpathipa and mjac0bs and removed request for a team August 21, 2024 15:50

Add changeset

230812b

carrillo-erik added the Requires Changes label Aug 21, 2024

Merge branch 'develop' of https://github.com/linode/manager into refa…

d3d2a98

…ctor/M3-6908

cpathipa reviewed Aug 22, 2024

View reviewed changes

carrillo-erik added 4 commits August 22, 2024 08:39

Revert changes to TimezoneForm

c9308f2

Fix more styling issues and mobile view

a176f78

Fix broken unit tests in CreateAPITokenDrawer

25983a1

Fix broken unit test in TimezoneForm

ff18e58

carrillo-erik removed the Requires Changes label Aug 23, 2024

Fix e2e test in Billing

9b8bd24

mjac0bs reviewed Aug 26, 2024

View reviewed changes

Fix failing e2e and console errors mentioned in PR review

438dc0c

carrillo-erik requested a review from a team as a code owner August 27, 2024 17:44

carrillo-erik requested review from cliu-akamai and removed request for a team August 27, 2024 17:44

Fix styling issues from PR review

ec934c1

carrillo-erik requested review from cpathipa and jaalah-akamai August 28, 2024 19:06

cpathipa reviewed Aug 29, 2024

View reviewed changes

carrillo-erik added the Ready for Review label Aug 29, 2024

carrillo-erik added 2 commits September 4, 2024 09:34

Merge branch 'develop' of https://github.com/linode/manager into refa…

dc365cf

…ctor/M3-6908

Fix selected option highlight in Question Component

f5889f2

mjac0bs self-requested a review September 6, 2024 19:02

mjac0bs reviewed Sep 6, 2024

View reviewed changes

carrillo-erik added 2 commits September 11, 2024 05:38

Merge branch 'develop' of https://github.com/linode/manager into refa…

306dbd1

…ctor/M3-6908

Fix failing test, ui bug, and 3rd party auth bug

965139d

carrillo-erik requested review from mjac0bs and cpathipa September 11, 2024 20:14

carrillo-erik added the Add'tl Approval Needed Waiting on another approval! label Sep 12, 2024

mjac0bs approved these changes Sep 12, 2024

View reviewed changes

bnussman-akamai approved these changes Sep 12, 2024

View reviewed changes

carrillo-erik merged commit 38a0d2e into linode:develop Sep 13, 2024
18 of 19 checks passed

jdamore-linode mentioned this pull request Sep 19, 2024

fix: [M3-8602, M3-8617] - Fix Autocomplete popper re-rendering causing test failures #10976

Open

12 tasks

bnussman-akamai mentioned this pull request Sep 20, 2024

poc: Autocomplete Popover fix #10978

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: [M3-6908] - Replace react-select in Profile #10780

refactor: [M3-6908] - Replace react-select in Profile #10780

carrillo-erik commented Aug 13, 2024 •

edited

Loading

cpathipa left a comment

github-actions bot commented Aug 23, 2024 •

edited

Loading

mjac0bs left a comment

mjac0bs Aug 26, 2024

mjac0bs Aug 26, 2024

mjac0bs Aug 26, 2024

mjac0bs Sep 6, 2024

carrillo-erik Sep 11, 2024

carrillo-erik commented Aug 27, 2024

cpathipa left a comment

cpathipa Aug 29, 2024

carrillo-erik Sep 4, 2024

cpathipa Aug 29, 2024

carrillo-erik Sep 4, 2024

cpathipa Aug 29, 2024

mjac0bs Sep 6, 2024

carrillo-erik Sep 11, 2024

mjac0bs Sep 12, 2024

mjac0bs commented Sep 3, 2024

mjac0bs Sep 6, 2024

mjac0bs Sep 6, 2024

mjac0bs left a comment

refactor: [M3-6908] - Replace react-select in Profile #10780

refactor: [M3-6908] - Replace react-select in Profile #10780

Conversation

carrillo-erik commented Aug 13, 2024 • edited Loading

Description 📝

Changes 🔄

Target release date 🗓️

How to test 🧪

Verification steps

As an Author I have considered 🤔

cpathipa left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 23, 2024 • edited Loading

mjac0bs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carrillo-erik commented Aug 27, 2024

cpathipa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjac0bs commented Sep 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjac0bs left a comment

Choose a reason for hiding this comment

carrillo-erik commented Aug 13, 2024 •

edited

Loading

github-actions bot commented Aug 23, 2024 •

edited

Loading