LF-5272: Replace yes/no crop generated field with crop/animal toggle on custom revenue type form#4168
Conversation
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
There was a problem hiding this comment.
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?" |
There was a problem hiding this comment.
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 }) { |
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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.
|
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! 🙏 |
kathyavini
left a comment
There was a problem hiding this comment.
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? 😅
Description
EntityAssociationTogglecomponentCustomRevenueRadioswithEntityAssociationToggleisCropSalefunctionInputBaseLabelcomponent ⬇️Jira link: https://lite-farm.atlassian.net/browse/LF-5272
Type of change
How Has This Been Tested?
Checklist:
pnpm i18nto help with this)