LF-4086: Create pure action bar component #3140
Conversation
SayakaOno
changed the title
SayakaOno
requested review from
antsgar and
kathyavini
and removed request for
a team
| }: ActionMenuProps) => { | ||
| // If the className is not yet supported, ensure to add corresponding styles for the widths | ||
| // of the component and each iconGroup to the SCSS file. | ||
| const iconCountClassName = styles[`iconCount_${iconActions.length}`]; |
There was a problem hiding this comment.
I think we can make this work more generically without having to add a specific class and do the math for each number of icons, but it was kinda hard to explain in words so I branched off and tried it out in a branch of my own. I pushed it, the branch name isgeneric-styling-for-action-menu -- take a look and let me know what you think!
There was a problem hiding this comment.
Thank you for reviewing! I don't see any new commits in your branch, have you pushed them??
The width of the action menu needs to be 408px for 4 icon buttons, so I added the className, but look forward to seeing your solution!
There was a problem hiding this comment.
I didn't push 😅 lol! Just pushed them
There was a problem hiding this comment.
Thank you @antsgar!
I took a look at your code. I think the problems that we will have when not specifying the widths are:
- Without the component width, the component will get longer when label texts are long. (Also I followed Loïc's design that the component width should be
408pxfor four buttons.) - Without the width for each button, the space between buttons differ depending on the label texts.
What do you think?
There was a problem hiding this comment.
Ah, I see! In that case your approach seems good since it's truer to the design spec, let's keep that 🙂
|
@antsgar @kathyavini |
Description
/animalsand the rest to/imagesdirectly. Let me know if this is a bad idea)isCompactSideMenutoInventorycomponentActionMenuto the Inventory component (this was out of scope, but I wanted to make sure the component can be positioned as expected)(Joyce, the colour class names haven't changed! That was a false alarm, sorry)
Jira link: https://lite-farm.atlassian.net/browse/LF-4086
Type of change
How Has This Been Tested?
Checklist: