Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

AP-1335 Fix MenuItems not being truncated properly #158

Merged
merged 4 commits into from
Feb 14, 2020
Merged

AP-1335 Fix MenuItems not being truncated properly #158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2e3fe79
💄 update MenuItem to allow truncation
justinanastos Feb 13, 2020
2c29b01
Share `PerformUserInteraction` component across tests
justinanastos Feb 13, 2020
3ff8236
Hopefully fix flake in ConfirmationTooltip story
justinanastos Feb 13, 2020
727fd31
Add story for truncated menu item text
justinanastos Feb 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 23 additions & 11 deletions src/ConfirmationTooltip/ConfirmationTooltip.story.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,33 @@
import React from "react";
import userEvent from "@testing-library/user-event";
import { Button } from "../Button";
import { ConfirmationTooltip } from "../ConfirmationTooltip";
import { findByText } from "@testing-library/dom";
import { PerformUserInteraction } from "../shared/PerformUserInteraction";
import { storiesOf } from "@storybook/react";

storiesOf("ConfirmationTooltip", module)
.add("forced visible", () => {
return (
<div style={{ padding: 50 }}>
<ConfirmationTooltip
content="default content"
forceVisibleForTestingOnly
.add(
"forced visible",
() => {
return (
<PerformUserInteraction
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, will these always trigger the new state before the snapshot is taken? I like this a lot better than testing only props. Would love to use this pattern in agm as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be incorporated into AGM in https://github.com/mdg-private/engine-frontend/pull/2016.

While I think this is a step in the right direction, it is not an ideal solution still. Chromatic does not have an async rendering feature (https://docs.chromaticqa.com/resource-loading#asynchronous-rendering), so we have to manually add { chromatic: { delay: number } } as the second argument to .add. @timbotnik suggested I write a blog post on this and gently nudge the chromatic team to offer the ability to programmatically delay snapshots until something has happened, which I think we be awesome.

All that being said, I'm ok adding a 2 second delay to a chromatic capture since they all run in parallel and this would enable us to write less magic props to force component states.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To answer your question more succinctly

will these always trigger the new state before the snapshot is taken

No, it will not. We have to manually delay the snapshot with a best-guess time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okay, that makes more sense. I still really like this setup though! Wasnt a huge fan of test props for stories

callback={async () => {
userEvent.click(await findByText(document.body, "click"));
}}
>
<Button>click</Button>
</ConfirmationTooltip>
</div>
);
})
<div style={{ padding: 50 }}>
<ConfirmationTooltip content="default content">
<Button>click</Button>
</ConfirmationTooltip>
</div>
</PerformUserInteraction>
);
},
{
chromatic: { delay: 500 },
}
)
.add("interactive", () => {
return (
<div style={{ padding: 50 }}>
Expand Down
86 changes: 85 additions & 1 deletion src/Menu/Menu.story.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { findByText } from "@testing-library/dom";
import { Button } from "../Button";
import { colors } from "../colors";
import { Menu } from "../Menu";
import { PerformUserInteraction } from "./menuStory/PerformUserInteraction";
import { PerformUserInteraction } from "../shared/PerformUserInteraction";
import { MenuItem } from "../MenuItem";
import { MenuDivider } from "../MenuDivider";
import { MenuHeading } from "../MenuHeading";
Expand Down Expand Up @@ -413,6 +413,90 @@ information and provide a nice visual grouping.
</Story>
</Preview>

## Truncation

Items should be truncated properly

export const MenuItemWithContentHuggingSides = ({
startContent,
endContent,
}) => {
return (
<div style={{ width: "100%", display: "flex" }}>
<span
style={{
flex: 1,
overflow: "hidden",
textOverflow: "ellipsis",
whiteSpace: "nowrap",
}}
>
{startContent}
</span>
<span
style={{
marginLeft: ".5rem",
flex: "none",
color: colors.grey.light,
}}
>
{endContent}
</span>
</div>
);
};

<Preview>
<Story name="flexbox truncation">
<PerformUserInteraction
callback={async () => {
userEvent.click(await findByText(document.body, /open menu/i));
}}
>
<Menu
placement="bottom-start"
iconSize="small"
maxWidth={280}
content={
<React.Fragment>
<MenuHeading startIcon={null}>mdg-private-graphs</MenuHeading>
<MenuItem startIcon={null}>
<MenuItemWithContentHuggingSides
startContent="really_long_test_name_really_long_test_name"
endContent="0 rpm"
/>
</MenuItem>
<MenuItem startIcon={null} selected>
<MenuItemWithContentHuggingSides
startContent="engine"
endContent="1.9k rm"
/>
</MenuItem>
<MenuItem
startIcon={
<IconCheck
style={{
color: colors.blue.base,
width: "100%",
height: "100%",
}}
/>
}
>
<MenuItemWithContentHuggingSides
startContent="galaxy-eu-west"
endContent="0 rm"
/>
</MenuItem>
</React.Fragment>
}
>
<Button>Open Menu</Button>
</Menu>
</PerformUserInteraction>
</Story>
</Preview>

## Props

### `Menu`
Expand Down
2 changes: 2 additions & 0 deletions src/MenuItem/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ export const MenuItem = React.forwardRef<
<div
css={css({
flex: "1",
/* This is weird but it's necessary to truncate menu items */
minWidth: 0,
})}
>
{children}
Expand Down
0 ...Menu/menuStory/PerformUserInteraction.tsx → src/shared/PerformUserInteraction.tsx
View file
Open in desktop
File renamed without changes.