Bastien-and-Gauvain / monorepo

A chrome extension to help recruiters 1-click export LinkedIn profiles in their Notion databases.
https://chromewebstore.google.com/detail/recruitivity/gahcfmlnmednhnbkifdoffblfffpeljf
3 stars 0 forks source link

🐛 fix(front) PLAS-58: remove error in the console on the select input #41

Closed GauvainThery closed 8 months ago

GauvainThery commented 8 months ago

Context

image

Solution

I simply removed the value prop of the input.

Priority

High

bvelitchkine commented 8 months ago

Look at this gif. I pulled the branch to run local tests and I:

  1. Went on your profile
  2. Selected your gender
  3. Saved your profile
  4. Changed your gender on LinkedIn Values to "Female"
  5. Clicked on the toggle to switch to the values stored in Notion

As you can see, your gender remained "Female" when it should have switched to "Male" (i.e the value stored in Notion).

Am I still missing something?

initialValueBug

GauvainThery commented 8 months ago

I think it's the opposite that should be done 😬 Select entries must be controlled because of the toggle from LI to Notion values.

Say we hit the toggle to switch from LI values to Notion values. Then, that simple hit must change the values of all the fields (including select entries) in the form and change the display.

That is to say that select entries are controlled and therefore, that value should be used, not initialValue (cf. this thread on stackoverflow).

We can have them controlled but it will not fix this problem as when we click the toggle we're setting back the values from LinkedIn or from Notion no matter what you did before. IMHO I find it clearer for the UX of the app - if you change the form it's to click on save right afterwards. Otherwise we would have to make it slightly differently

Fixing the problem would you mentioned would require to change the way the form is built (we'll do for sure in the future)

GauvainThery commented 8 months ago

LGTM! Did you change your mind bc it turns out I was right, or bc you felt like this would be an endless talk? 🙃

For me both were right but the way we use the form, we might end up needing a controlled input 😉