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

LF-4351: Other purpose text is not saved #3344

Merged
merged 2 commits into from
Jul 29, 2024

Conversation

SayakaOno
Copy link
Collaborator

Description

  • Set ``OTHER_PURPOSE_ID` in useEffect.
  • Remove value and defaultValue from the input to avoid the controlled/uncontrolled warning.
  • Remove out-dated comment.

Jira link: https://lite-farm.atlassian.net/browse/LF-4351

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • Passes test case
  • UI components visually reviewed on desktop view
  • UI components visually reviewed on mobile view
  • Other (please explain)

Checklist:

  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • The precommit and linting ran successfully
  • I have added or updated language tags for text that's part of the UI
  • I have added "MISSING" for all new language tags to languages I don't speak
  • I have added the GNU General Public License to all new files

@SayakaOno SayakaOno added the bug Something isn't working label Jul 29, 2024
@SayakaOno SayakaOno self-assigned this Jul 29, 2024
@SayakaOno SayakaOno requested review from a team as code owners July 29, 2024 17:27
@SayakaOno SayakaOno requested review from Duncan-Brain and kathyavini and removed request for a team July 29, 2024 17:27
Copy link
Collaborator

@kathyavini kathyavini left a comment

Choose a reason for hiding this comment

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

I have never been able to get this behaviour happening locally to test, but the code change makes sense!

@SayakaOno
Copy link
Collaborator Author

Thank you @kathyavini ! The reproduction rate is 100% for me so far; it needs to be started without purposes in the Redux store (testing right after logging in is the easiest), and if you run it with a throttle, you might be able to see it too. (But no need to re-test :) )

Copy link
Collaborator

@Duncan-Brain Duncan-Brain left a comment

Choose a reason for hiding this comment

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

Fixes it for me!

@Duncan-Brain Duncan-Brain merged commit f6b3b17 into patch/3.6.6 Jul 29, 2024
@kathyavini
Copy link
Collaborator

@SayakaOno ah okay thank you! With throttling I was able to see the query still pending with the page loaded and the value missing!! I was thinking of this in the wrong way entirely then because I had assumed the undefined was just the form's initial load behaviour and not the actual value being missing (or if I had known about the missing value, I had forgotten by today 😂)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants