Skip to content

Commit

Permalink
fix(component): display text only on Sidebar.Item tooltip (#315)
Browse files Browse the repository at this point in the history
* fix(component): display text only on `Sidebar.Item` tooltip

When a `Sidebar` is collapsed, each item's text content is hidden. You hover over an item to see a
`Tooltip` with its text content. That isn't how it worked up until now.

resolve #258

* test(component): drop false-positive `Sidebar` tests

I don't think we can rely on a fake DOM environment to test hover states correctly. We should write
integration tests for `<Sidebar collapsed/>` behavior in `cypress`.
  • Loading branch information
tulup-conner authored Jul 31, 2022
1 parent 77cb356 commit 9af5d13
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 45 deletions.
1 change: 0 additions & 1 deletion src/lib/components/Flowbite/FlowbiteTheme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,6 @@ export interface FlowbiteTheme {
};
content: {
base: string;
collapsed: string;
};
icon: {
base: string;
Expand Down
27 changes: 0 additions & 27 deletions src/lib/components/Sidebar/Sidebar.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -117,30 +117,6 @@ describe.concurrent('Components / Sidebar', () => {
});
});

describe('`Sidebar.Collapse` and `Sidebar.Item`', () => {
it('should display tooltip', () => {
const items = getSidebarItems(render(<TestSidebar collapsed />));

items.forEach((item) => {
expect(item.firstElementChild).toHaveAttribute('data-testid', 'tooltip-target');
});
});

it("shouldn't display text content", () => {
const items = getSidebarItemContents(render(<TestSidebar collapsed />));

items.forEach((item) => expect(item).toHaveClass('hidden'));
});
});

describe('`Sidebar.Item`', () => {
it("shouldn't display `label`", () => {
const labels = getSidebarLabels(render(<TestSidebar collapsed />));

labels.forEach((label) => expect(label).not.toBeVisible());
});
});

describe('`Sidebar.Logo`', () => {
it("shouldn't display text content", () => {
const logo = getSidebarLogo(render(<TestSidebar collapsed />));
Expand Down Expand Up @@ -459,8 +435,5 @@ const getSidebarItems = ({ getAllByRole }: Pick<RenderResult, 'getAllByRole'>):
const getSidebarItemsContainer = ({ getAllByTestId }: Pick<RenderResult, 'getAllByTestId'>): HTMLElement[] =>
getAllByTestId('flowbite-sidebar-items');

const getSidebarLabels = ({ queryAllByTestId }: Pick<RenderResult, 'queryAllByTestId'>): HTMLElement[] =>
queryAllByTestId('flowbite-sidebar-label');

const getSidebarLogo = ({ getByLabelText }: Pick<RenderResult, 'getByLabelText'>): HTMLElement =>
getByLabelText('Flowbite');
2 changes: 1 addition & 1 deletion src/lib/components/Sidebar/Sidebar.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ CTAButton.args = {
</Sidebar.Items>
<Sidebar.CTA>
<div className="mb-3 flex items-center">
<Badge color="yellow">Beta</Badge>
<Badge color="warning">Beta</Badge>
<div className="-m-1.5 ml-auto">
<Button aria-label="Close" outline>
<svg className="h-4 w-4" fill="currentColor" viewBox="0 0 20 20" xmlns="http://www.w3.org/2000/svg">
Expand Down
30 changes: 17 additions & 13 deletions src/lib/components/Sidebar/SidebarItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,26 +32,36 @@ const SidebarItem: FC<SidebarItemProps> = ({
label,
labelColor = 'info',
...props
}): JSX.Element => {
}) => {
const theirProps = excludeClassName(props);

const id = useId();
const { isCollapsed } = useSidebarContext();
const { isInsideCollapse } = useSidebarItemContext();
const theme = useTheme().theme.sidebar.item;

const Wrapper: FC<PropsWithChildren<unknown>> = ({ children }): JSX.Element => (
const Wrapper: FC<PropsWithChildren<unknown>> = ({ children: Component }) => (
<li>
{isCollapsed ? (
<Tooltip content={children} placement="right">
{children}
<Tooltip content={<Children>{children}</Children>} placement="right">
{Component}
</Tooltip>
) : (
children
Component
)}
</li>
);

const Children: FC<PropsWithChildren<unknown>> = ({ children }) => (
<span
className={classNames(theme.content.base)}
data-testid="flowbite-sidebar-item-content"
id={`flowbite-sidebar-item-${id}`}
>
{children}
</span>
);

return (
<Wrapper>
<Component
Expand All @@ -70,14 +80,8 @@ const SidebarItem: FC<SidebarItemProps> = ({
data-testid="flowbite-sidebar-item-icon"
/>
)}
<span
className={classNames(theme.content.base, isCollapsed && theme.content.collapsed)}
data-testid="flowbite-sidebar-item-content"
id={`flowbite-sidebar-item-${id}`}
>
{children}
</span>
{label && (
{!isCollapsed && <Children>{children}</Children>}
{label && !isCollapsed && (
<Badge color={labelColor} data-testid="flowbite-sidebar-label" hidden={isCollapsed}>
{label}
</Badge>
Expand Down
2 changes: 1 addition & 1 deletion src/lib/components/Sidebar/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import classNames from 'classnames';
import type { ComponentProps, FC, PropsWithChildren } from 'react';
import { ComponentProps, FC, PropsWithChildren } from 'react';
import { excludeClassName } from '../../helpers/exclude';
import { useTheme } from '../Flowbite/ThemeContext';
import SidebarCollapse from './SidebarCollapse';
Expand Down
3 changes: 1 addition & 2 deletions src/lib/theme/default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -697,8 +697,7 @@ export default {
insideCollapse: 'group w-full pl-8 transition duration-75',
},
content: {
base: 'ml-3 flex-1 whitespace-nowrap',
collapsed: 'hidden',
base: 'px-3 flex-1 whitespace-nowrap',
},
icon: {
base: 'h-6 w-6 flex-shrink-0 text-gray-500 transition duration-75 group-hover:text-gray-900 dark:text-gray-400 dark:group-hover:text-white',
Expand Down

0 comments on commit 9af5d13

Please sign in to comment.