Skip to content

LF-5272: Replace yes/no crop generated field with crop/animal toggle on custom revenue type form#4168

Merged
kathyavini merged 20 commits into
integrationfrom
LF-5272/Replace_yes/no_crop_generated_field_with_crop/animal_toggle_on_custom_revenue_type_form
May 15, 2026
Merged

LF-5272: Replace yes/no crop generated field with crop/animal toggle on custom revenue type form#4168
kathyavini merged 20 commits into
integrationfrom
LF-5272/Replace_yes/no_crop_generated_field_with_crop/animal_toggle_on_custom_revenue_type_form

Conversation

@SayakaOno
Copy link
Copy Markdown
Collaborator

@SayakaOno SayakaOno commented May 11, 2026

Description

  • Add EntityAssociationToggle component
  • Replace the radio buttons in CustomRevenueRadios with EntityAssociationToggle
  • Replace 'crop_generated' with 'entity_type' and remove 'agriculture_associated'
  • Add isCropSale function
  • Remove temporary compatibility shims from sagas
  • Add gap to InputBaseLabel component ⬇️
    Screenshot 2026-05-11 at 10 26 03 AM

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

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 ordered translation keys alphabetically (optional: run pnpm i18n to help with this)
  • I have added the GNU General Public License to all new files

SayakaOno and others added 12 commits May 8, 2026 12:34
Replace crop_generated/agriculture_associated with entity_type on RevenueType.
Add EntityType, AnimalSaleItem, FarmExpenseCropVariety, FarmExpenseAnimal types.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… consumers

Remove the saga bridge that converted entity_type → crop_generated.
Update revenueTypeSlice pick list, saga payload handling, util.js helpers,
and useTransactions to use entity_type === 'crop' directly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two-button toggle (Crops/Animals) with deselect support. EntityToggleButton
defined in the same file. Uses InputBaseLabel for the label/optional pattern.
Adds REVENUE.ENTITY_ASSOCIATION i18n keys to the English locale.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace CROP_GENERATED constant with ENTITY_TYPE. Swap RadioGroup in
CustomRevenueRadios for EntityAssociationToggle (via useController).
Update Edit/ReadOnly views to default to entity_type, showing
'Not associated' when no selection in read-only mode.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…w, test, and story

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…_field_with_crop/animal_toggle_on_custom_revenue_type_form
@SayakaOno SayakaOno self-assigned this May 11, 2026
@SayakaOno SayakaOno added enhancement New feature or request new translations New translations to be sent to CrowdIn are present labels May 11, 2026
@SayakaOno SayakaOno marked this pull request as ready for review May 13, 2026 19:06
@SayakaOno SayakaOno requested review from a team as code owners May 13, 2026 19:06
@SayakaOno SayakaOno requested review from kathyavini and removed request for a team May 13, 2026 19:06
Copy link
Copy Markdown
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.

This looks (and behaves -- I've been using the draft state as my base branch for the last revenue ticket) great!

Personally I think it could deserve its own Storybook story, since it has a few states and will get used in two contexts, but it's up to you 🙂

Other that that, just a few super tiny things below.

AI review pass flagged the accessibility of the toggle (particularly reading the selected state to screen readers now that it's a button), but I don't think it will be the easiest fix and of course screen reader accessibility is a huge gap in app already...

"DUPLICATE_NAME": "A revenue type with this name already exists. Please choose another.",
"DUPLICATE_NAME_RETIRED": "A retired revenue type with this name already exists. Please choose another."
"DUPLICATE_NAME_RETIRED": "A retired revenue type with this name already exists. Please choose another.",
"ENTITY_ASSOCIATION_LABEL": "Is this revenue associated with animals or crop?"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this is a mistake on Figma; it should be "crops" to be both grammatical and accurate.

*/

function CustomRevenueRadios({ control, watch, view }) {
function CustomRevenueRadios({ control, view }) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does this component need another name, now that there is no <RadioGroup> here? Something a bit more general like CustomRevenueEntityTypeSelector or such?

@include truncateText();
}

&:hover {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This may be a good use case for the @media (hover: hover) pattern, because on mobile when you tap, and then tap again to deselect, this is the state that persists, giving a sort of false 'active' look.

@SayakaOno
Copy link
Copy Markdown
Collaborator Author

Thank you Joyce for the comments, and I'm sorry I forgot this was blocking your ticket...

I thought about creating a story as well, but I didn't think it would be very useful, so I decided not to.

Personally, accessibility is definitely something I'd like to get better at, but I'll leave it for now. I wasn't even sure how to classify that component, de-selectable radio buttons? Checkboxes where only one option can be selected? Anyways...

Thank you for re-reviewing! 🙏

Copy link
Copy Markdown
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 wasn't even sure how to classify that component, de-selectable radio buttons? Checkboxes where only one option can be selected?

Now that you say it, it's kind of weird isn't it? 😅

@kathyavini kathyavini added this pull request to the merge queue May 15, 2026
Merged via the queue into integration with commit 33feb49 May 15, 2026
6 checks passed
@SayakaOno SayakaOno deleted the LF-5272/Replace_yes/no_crop_generated_field_with_crop/animal_toggle_on_custom_revenue_type_form branch May 15, 2026 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request new translations New translations to be sent to CrowdIn are present

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants