LF-4481: Add Location to animal inventory UI #3527
Conversation
SayakaOno
changed the title
SayakaOno
changed the title
SayakaOno
force-pushed
the
LF-4481/Add_Location_to_animal_inventory_UI
branch
from
9e1037f
to
ff9c721
Compare
SayakaOno
changed the base branch from
integration
to
LF-4384/End-to-End_Animal_Details
* add icon * show 'Unknown' for animals without location
* adjust stories
SayakaOno
force-pushed
the
LF-4481/Add_Location_to_animal_inventory_UI
branch
from
ff9c721
to
1da9b44
Compare
|
Looks/works great, it's all coming together!! 🙌 |
There was a problem hiding this comment.
Everything works great!
I might have preferred to keep the PLAIN cell kind very text specific, and adapt ICON_TEXT instead, or not use standardized Cell at all. <Cell /> is not necessary if making something single use and custom. But I will leave it up to you!
| const Plain = ({ text }: PlainCellProps) => { | ||
| return <div className={clsx(styles.text, styles.overflowText)}>{text}</div>; | ||
| const Plain = ({ content }: PlainCellProps) => { | ||
| return <div className={clsx(styles.text, styles.overflowText)}>{content}</div>; |
There was a problem hiding this comment.
There are also these styles which were meant to only specified for text.
| label: t('ANIMAL.ANIMAL_LOCATIONS').toLocaleUpperCase(), | ||
| format: (d: AnimalInventory) => ( | ||
| <Cell | ||
| kind={CellKind.PLAIN} |
There was a problem hiding this comment.
There is a cell kind of "ICON_TEXT" did it not work?
There was a problem hiding this comment.
I just tried it out. We would need to work around the text (the component requires ICON_NAME, and the location needs to be the text Unknown for undefined locations) and pass custom styles. I think it's better not to use the Cell.
There was a problem hiding this comment.
Yup up to you!
I think
d.location ? <Cell kind={ICONTEXT} text iconName ../> : <Cell kind={PLAIN} text />
Would also work to keep text styles same across cells but I am good with anything.
There was a problem hiding this comment.
The location text style is unique 😅
There was a problem hiding this comment.
Hmm 🤔 . Yeah, it is unique.... Is it designed to be different than all other cells? It seems strange to me to have one cell have different size, color, and weight of text.
The whole point of <Cell /> was to enforce the same text and other styles -- its almost useless otherwise haha.
There was a problem hiding this comment.
I believe the difference is intentional. I don’t have a strong sense of design, but it doesn’t seem strange to me :)
|
Thank you for your input Duncan! I've reverted the changes to the Plain cell component and updated the location column to avoid using |
Description
Inventory table
location_idto Animal and AnimalBatch types and adjust mockDatauseAnimalInventoryto include locationuseFilteredInventoryto handle locations filterlocationstring to animal inventory searchable stringPlaincell component to takecontent(ReactNode) for greater flexibilityHeader
AnimalSingleViewHeader's props (separatedlocationTextfromanimalOrBatch) and adjust storiesAnimalSingleViewHeaderJira link: https://lite-farm.atlassian.net/browse/LF-4481
Type of change
How Has This Been Tested?
Checklist: