feat: add the selectedIcon prop to the SidebarItem component #1574
Conversation
| return ( | ||
| <Fragment> | ||
| <StyledIcon>{icon}</StyledIcon> | ||
| {isSelected ? <StyledIcon>{selectedIcon}</StyledIcon> : <StyledIcon>{icon}</StyledIcon>} |
There was a problem hiding this comment.
normally we use RenderIf for all internal conditionals of the lib
src/components/SidebarItem/readme.md
Outdated
| <SidebarItem icon={<DashboardPurpleIcon />} name="Dashboard" label="Dashboard" /> | ||
| <SidebarItem icon={<ApplicationIcon />} name="Aplications" label="Aplications" /> | ||
| <SidebarItem icon={<PuzzleIcon />} name="Components" label="Components" /> | ||
| <SidebarItem icon={<MessagesIcon />} name="Messages" label="Messages" /> | ||
| <SidebarItem | ||
| icon={<DashboardPurpleIcon />} | ||
| selectedIcon={<UserIcon />} | ||
| name="Dashboard" |
There was a problem hiding this comment.
I think you should leave the example simple and add a new one with the selectedIcon
src/components/SidebarItem/index.js
Outdated
| const isSelected = name === selectedItem; | ||
| const selectIcon = selectedIcon || icon; |
There was a problem hiding this comment.
I was looking and I think you could define the icon here
const currentIcon = isSelected && !!selectedIcon ? selectedIcon : icon;
src/components/SidebarItem/index.js
Outdated
| icon={icon} | ||
| selectedIcon={selectIcon} |
There was a problem hiding this comment.
Here it does "icon = {currentIcon}" and you don't have to change the ItemContent
src/components/SidebarItem/index.js
Outdated
| icon={icon} | ||
| selectedIcon={selectIcon} |
| export default function ItemContent(props) { | ||
| const { label, icon, isSelected } = props; | ||
| const { label, icon, selectedIcon, isSelected } = props; | ||
| return ( | ||
| <Fragment> | ||
| <StyledIcon>{icon}</StyledIcon> | ||
| <RenderIf isTrue={!!isSelected}> | ||
| <StyledIcon>{selectedIcon}</StyledIcon> | ||
| </RenderIf> | ||
| <RenderIf isTrue={!isSelected}> | ||
| <StyledIcon>{icon}</StyledIcon> | ||
| </RenderIf> |
src/components/SidebarItem/readme.md
Outdated
| <Sidebar selectedItem={selectedItem} onSelect={this.handleOnSelect} id="sidebar-1"> | ||
| <SidebarItem icon={<DashboardPurpleIcon />} name="Dashboard" label="Dashboard" /> | ||
| <SidebarItem icon={<ApplicationIcon />} name="Aplications" label="Aplications" /> | ||
| <SidebarItem icon={<PuzzleIcon />} name="Components" label="Components" /> | ||
| <SidebarItem icon={<MessagesIcon />} name="Messages" label="Messages" /> | ||
| <SidebarItem | ||
| icon={<DashboardPurpleIcon />} |
There was a problem hiding this comment.
I think you should leave the example simple and add a new one with the selectedIcon
| ); | ||
| const sidebarLink = component.find('a'); | ||
| sidebarLink.simulate('click'); | ||
| expect(component.find({ 'data-icon': 'clock' }).exists()).toBe(true); |
There was a problem hiding this comment.
This is something internal from FontAwesome? If true then we need to change this assertion since we can't make introspection inside what we could not control, font awesome can change this behaviour and then if we update then our test take the risk to fail, instead, we should find the FontAwesomeIcon component in the tree of react components and check that the icon prop passed is faClock
fix: #1544
Changes proposed in this PR:
add the
selectedIconprop to theSidebarItemcomponentI have followed (at least) the PR section of the contributing guide.
@nexxtway/react-rainbow