openedx / frontend-app-authn

Open edX micro-frontend application for new login and registration experience.
GNU Affero General Public License v3.0
7 stars 153 forks source link

update `CountryField` to better utilize new paragon `Form.Autosuggest` #1133

Open brian-smith-tcril opened 10 months ago

brian-smith-tcril commented 10 months ago

Paragon 22 includes breaking changes for Form.Autosuggest. These should aid in simplifying the implementation of CountryField, but some changes may be needed in Paragon to support certain parts of the error handling logic. One part that stands out is handleErrorChange as seen here:

https://github.com/openedx/frontend-app-authn/blob/2ea9301c5e763b26e6e4529e9ff2c3283b0f6bdf/src/register/RegistrationFields/CountryField/CountryField.jsx#L68-L68

brian-smith-tcril commented 10 months ago

add callback "onErrorStateChange" to Form.Autosuggest

brian-smith-tcril commented 9 months ago

As of https://github.com/openedx/frontend-app-authn/pull/1157 the new Autosuggest component is being used, but not to its full potential

None of the internal error state management is being used

https://github.com/openedx/frontend-app-authn/blob/a6e96f5ed1ac83c999e0da78e8639687fac88756/src/register/RegistrationFields/CountryField/CountryField.jsx#L107-L116

instead the error state is managed in handleOnFocus and handleOnBlur

https://github.com/openedx/frontend-app-authn/blob/a6e96f5ed1ac83c999e0da78e8639687fac88756/src/register/RegistrationFields/CountryField/CountryField.jsx#L67-L85

and there are workarounds such as

https://github.com/openedx/frontend-app-authn/blob/a6e96f5ed1ac83c999e0da78e8639687fac88756/src/register/RegistrationFields/CountryField/CountryField.jsx#L90-L96

The comment mentions the lack of onSelected, but handleOnSelected would just call handleOnBlur before

https://github.com/openedx/frontend-app-authn/blob/872aa4867540d5d45605ba1699a0007595edf83b/src/register/RegistrationFields/CountryField/CountryField.jsx#L76-L78

which, as of https://github.com/openedx/frontend-app-authn/pull/1157, is just used for updating error state

-    const { countryCode, displayValue, error } = validateCountryField(
+    const { error } = validateCountryField(
       value.trim(), countryList, formatMessage(messages['empty.country.field.error']), formatMessage(messages['invalid.country.field.error']),
     );
-
-    onChangeHandler({ target: { name: 'country' } }, { countryCode, displayValue });
     handleErrorChange('country', error);

Therefore, I believe that this can be handled with an onErrorStateChange callback in Form.Autosuggest (https://github.com/openedx/paragon/issues/3002)

zainab-amir commented 9 months ago

Hi @brian-smith-tcril, Thank you for this detailed comment.

In addition to the missing onErrorStateChange, the reason for not using the internal error state provided by Form.Autosuggest is the way it renders error. It is prefixed with "x" icon with no way to remove it. All other errors on Authn don't have it and this makes the form errors inconsistent.

AutoSuggest Error Other Errors on Authn
Screenshot 2024-02-13 at 12 55 14 PM Screenshot 2024-02-13 at 12 55 46 PM

We will resume working on this and update country field once we have onErrorStateChange. This will provide us a way to set multiple errors i.e empty field error and invalid selection error