From 39691182bd5352a18d0c46da3297e726d8f28f30 Mon Sep 17 00:00:00 2001 From: Hadas Farhi Date: Wed, 12 Jan 2022 11:05:56 +0200 Subject: [PATCH 01/11] flex component --- src/components/Flex/Flex.jsx | 93 +++++++++++ src/components/Flex/Flex.module.scss | 91 +++++++++++ src/components/Flex/FlexConstants.js | 7 + .../Flex/__stories__/Flex.stories.mdx | 144 ++++++++++++++++++ .../Flex/__stories__/Flex.stories.scss | 5 + .../__tests__/flex-snapshot-tests.jest.js | 50 ++++++ .../Flex/__tests__/flex-tests.jest.js | 19 +++ src/components/index.js | 2 + src/constants/positions.js | 5 + src/constants/sizes.js | 6 +- 10 files changed, 419 insertions(+), 3 deletions(-) create mode 100644 src/components/Flex/Flex.jsx create mode 100644 src/components/Flex/Flex.module.scss create mode 100644 src/components/Flex/FlexConstants.js create mode 100644 src/components/Flex/__stories__/Flex.stories.mdx create mode 100644 src/components/Flex/__stories__/Flex.stories.scss create mode 100644 src/components/Flex/__tests__/flex-snapshot-tests.jest.js create mode 100644 src/components/Flex/__tests__/flex-tests.jest.js create mode 100644 src/constants/positions.js diff --git a/src/components/Flex/Flex.jsx b/src/components/Flex/Flex.jsx new file mode 100644 index 0000000000..7fe65ca64b --- /dev/null +++ b/src/components/Flex/Flex.jsx @@ -0,0 +1,93 @@ +import React, { useRef, forwardRef } from "react"; +import PropTypes from "prop-types"; +import cx from "classnames"; +import useMergeRefs from "../../hooks/useMergeRefs"; +import { FLEX_POSITIONS } from "./FlexConstants"; +import { BASE_SIZES } from "../../constants/sizes"; +import classes from "./Flex.module.scss"; + +const Flex = forwardRef( + ( + { + className, + id, + vertical, + wrap, + children, + horizontalPosition, + verticalPosition, + horizontalSpacingSize, + verticalSpacingSize + }, + ref + ) => { + const componentRef = useRef(null); + const mergedRef = useMergeRefs({ refs: [ref, componentRef] }); + + return ( +
+ {children} +
+ ); + } +); + +Flex.horizontalPositions = FLEX_POSITIONS; +Flex.verticalPositions = FLEX_POSITIONS; +Flex.verticalSpacingSizes = BASE_SIZES; +Flex.horizontalSpacingSizes = BASE_SIZES; +Flex.propTypes = { + /** + * class name to be add to the wrapper + */ + className: PropTypes.string, + /** + * id to be add to the wrapper + */ + id: PropTypes.string, + vertical: PropTypes.bool, + wrap: PropTypes.bool, + children: PropTypes.element, + horizontalPosition: PropTypes.oneOf([ + Flex.horizontalPositions.START, + Flex.horizontalPositions.CENTER, + Flex.horizontalPositions.END, + Flex.horizontalPositions.SPACE_BETWEEN, + Flex.horizontalPositions.SPACE_AROUND + ]), + verticalPosition: PropTypes.oneOf([ + Flex.verticalPositions.START, + Flex.verticalPositions.CENTER, + Flex.verticalPositions.END, + Flex.verticalPositions.SPACE_BETWEEN, + Flex.verticalPositions.SPACE_AROUND + ]), + verticalSpacingSize: + PropTypes.oneOf[ + (Flex.verticalSpacingSizes.SMALL, Flex.verticalSpacingSizes.MEDIUM, Flex.verticalSpacingSizes.LARGE) + ] +}; + +Flex.defaultProps = { + className: "", + id: undefined, + vertical: false, + wrap: false, + children: undefined +}; + +export default Flex; diff --git a/src/components/Flex/Flex.module.scss b/src/components/Flex/Flex.module.scss new file mode 100644 index 0000000000..5a79e0ac5d --- /dev/null +++ b/src/components/Flex/Flex.module.scss @@ -0,0 +1,91 @@ +@import "../../styles/themes.scss"; + +@mixin main-position-classes { + &-start { + justify-content: flex-start; + } + + &-end { + justify-content: flex-end; + } + + &-center { + justify-content: center; + } + + &-space-between { + justify-content: space-between; + } + + &-space-around { + justify-content: space-around; + } +} +@mixin secondary-position-classes { + &-start { + align-content: flex-start; + } + + &-end { + align-content: flex-end; + } + + &-center { + align-content: center; + } + + &-space-between { + align-content: space-between; + } + + &-space-around { + align-content: space-around; + } +} + +.flex-container { + display: flex; + flex-direction: row; + & .horizontal { + &-position { + @include main-position-classes; + } + &-spacing-size { + &-small { + & > * { + margin-inline-end: var(--spacing-small); + } + } + &-medium { + & > * { + margin-inline-end: var(--spacing-medium); + } + } + &-large { + & > * { + margin-inline-end: var(--spacing-large); + } + } + } + } + + &.vertical { + &-flex-container { + flex-direction: column; + + & .vertical-position { + @include main-position-classes; + } + + & .horizontal-position { + @include secondary-position-classes; + } + } + &-position { + @include secondary-position-classes; + } + &-spacing-size { + @include spacing-size-classes(block-end) + } + } +} \ No newline at end of file diff --git a/src/components/Flex/FlexConstants.js b/src/components/Flex/FlexConstants.js new file mode 100644 index 0000000000..4a1b36ea0b --- /dev/null +++ b/src/components/Flex/FlexConstants.js @@ -0,0 +1,7 @@ +import { BASE_POSITIONS } from "../../constants/positions"; + +export const FLEX_POSITIONS = Object.freeze({ + ...BASE_POSITIONS, + SPACE_AROUND: "space-around", + SPACE_BETWEEN: "space-between" +}); diff --git a/src/components/Flex/__stories__/Flex.stories.mdx b/src/components/Flex/__stories__/Flex.stories.mdx new file mode 100644 index 0000000000..bdf0a7108e --- /dev/null +++ b/src/components/Flex/__stories__/Flex.stories.mdx @@ -0,0 +1,144 @@ +import Flex from "../Flex"; +import { ArgsTable, Story, Canvas, Meta } from "@storybook/addon-docs"; +import { createComponentTemplate } from "../../../storybook/functions/create-component-story"; +import "./Flex.stories.scss"; +import Icon from "../../Icon/Icon"; +import { Add, Close, Team } from "../../Icon/Icons"; + + + + + +export const flexTemplate = (args) => { + return + + + + +} + + + +# Flex +- [Overview](#overview) +- [Props](#props) +- [Usage](#usage) +- [Variants](#variants) +- [Do’s and don’ts](#dos-and-donts) +- [Use cases and examples](#use-cases-and-examples) +- [Related components](#related-components) +- [Feedback](#feedback) + +## Overview +Use Flex component to position group of sub-elements in one dimension, horizontal or vertical, without being dependent on a custom CSS file for positioning the sub-elements. + + + + { flexTemplate.bind({}) } + + + +## Props + + +## Usage + + +Tip content + +## Variants +### Story title 1 +Description of story 1 + + + + + + + +### Story title 2 +Description of story 2 + + + + + + + +## Do’s and Don’ts +, + description:"Best practice description." + }, + negative: { + component:, + description:"Bad practice description." + } + }, + { + positive: { + component:, + description:"Best practice description." + }, + negative: { + component:, + description:"Bad practice description." + } + }, + { + positive: { + component:, + description:"Best practice description." + }, + negative: { + component:, + description:"Bad practice description." + } + }, + { + positive: { + component:, + description:"Best practice description." + }, + negative: { + component:, + description:"Bad practice description." + } + } + ]} +/> + + +## Use cases and examples + +### Story title 3 +Description of story 3 + + + + + + + +### Story title 4 +Description of story 4 + + + + + + + +## Related components + \ No newline at end of file diff --git a/src/components/Flex/__stories__/Flex.stories.scss b/src/components/Flex/__stories__/Flex.stories.scss new file mode 100644 index 0000000000..cfecda8b15 --- /dev/null +++ b/src/components/Flex/__stories__/Flex.stories.scss @@ -0,0 +1,5 @@ +@import "../../../styles/themes.scss"; + +.monday-storybook-flex { + +} diff --git a/src/components/Flex/__tests__/flex-snapshot-tests.jest.js b/src/components/Flex/__tests__/flex-snapshot-tests.jest.js new file mode 100644 index 0000000000..5846db0f4b --- /dev/null +++ b/src/components/Flex/__tests__/flex-snapshot-tests.jest.js @@ -0,0 +1,50 @@ +import React from "react"; +import renderer from "react-test-renderer"; +import Flex from "../Flex"; + +/** + * There are cases where the component we want to test in the snapshot test will contain additional components. + We do not want changes to the additional components to fail our component snapshot's test. + Therefore, we will replace the instances of the other external components in the snapshot test with mock/stub components in these cases. + */ + +/** example for external library + jest.mock("react-transition-group", () => { + const FakeTransition = jest.fn(({ children }) => children); + const FakeSwitchTransition = jest.fn(({ children }) => children); + const FakeCSSTransition = jest.fn(({ children }) => children); + + // We return here the instance of the mock / stub library object content + return { + CSSTransition: FakeCSSTransition, + Transition: FakeTransition, + SwitchTransition: FakeSwitchTransition + }; +}); + **/ + +/** example for internal component +jest.mock("../../Button/Button", () => { + // We return here the instance of the mock / stub component + return ({ onClick }) => ( +
+ ); +}); +**/ + +describe("Flex renders correctly", () => { + it("with empty props", () => { + const tree = renderer.create().toJSON(); + expect(tree).toMatchSnapshot(); + }); + + it("with x", () => { + const tree = renderer.create().toJSON(); + expect(tree).toMatchSnapshot(); + }); + + it("with y", () => { + const tree = renderer.create().toJSON(); + expect(tree).toMatchSnapshot(); + }); +}); \ No newline at end of file diff --git a/src/components/Flex/__tests__/flex-tests.jest.js b/src/components/Flex/__tests__/flex-tests.jest.js new file mode 100644 index 0000000000..da0d50b107 --- /dev/null +++ b/src/components/Flex/__tests__/flex-tests.jest.js @@ -0,0 +1,19 @@ +import React from "react"; +import { fireEvent, render, cleanup } from "@testing-library/react"; +import { act } from "@testing-library/react-hooks"; +import Flex from "../Flex"; + + +const renderComponent = props => { + return render(); +}; + +describe("Flex tests", () => { + afterEach(() => { + cleanup(); + }); + + it("calls x when y", () => {}); + + it("should do x when y", () => {}); +}); diff --git a/src/components/index.js b/src/components/index.js index f1dd2ca699..e9fccd1061 100644 --- a/src/components/index.js +++ b/src/components/index.js @@ -71,3 +71,5 @@ export { default as Accordion } from "./Accordion/Accordion/Accordion"; export { default as AccordionItem } from "./Accordion/AccordionItem/AccordionItem"; export { default as Clickable } from "./Clickable/Clickable"; export { default as ColorUtils } from "../utils/colors-utils"; + +export { default as Flex } from "./Flex/Flex"; \ No newline at end of file diff --git a/src/constants/positions.js b/src/constants/positions.js new file mode 100644 index 0000000000..b73880a2ba --- /dev/null +++ b/src/constants/positions.js @@ -0,0 +1,5 @@ +export const BASE_POSITIONS = { + START: "start", + CENTER: "center", + END: "end" +}; diff --git a/src/constants/sizes.js b/src/constants/sizes.js index 4b1dd73d7f..b71240a0b8 100644 --- a/src/constants/sizes.js +++ b/src/constants/sizes.js @@ -1,9 +1,9 @@ +export const BASE_SIZES = Object.freeze({ SMALL: "small", MEDIUM: "medium", LARGE: "large" }); + export const SIZES = Object.freeze({ XXS: "xxs", XS: "xs", - SMALL: "small", - MEDIUM: "medium", - LARGE: "large" + ...BASE_SIZES }); export const DialogPositions = Object.freeze({ From 40006431e08e7c7c978ff3f8f29ad15cc7a0ac5e Mon Sep 17 00:00:00 2001 From: Hadas Farhi Date: Thu, 13 Jan 2022 14:22:25 +0200 Subject: [PATCH 02/11] flex component --- src/components/Flex/Flex.jsx | 35 ++- src/components/Flex/Flex.module.scss | 36 ++- src/components/Flex/FlexConstants.js | 6 + .../Flex/__stories__/Flex.stories.mdx | 296 +++++++++++++----- .../Flex/__stories__/Flex.stories.module.scss | 11 + .../Flex/__stories__/Flex.stories.scss | 5 - .../component-description-map.js | 3 + .../related-components/descriptions/list.jsx | 29 ++ .../story-description/story-description.jsx | 34 ++ .../story-description.module.scss | 4 + 10 files changed, 350 insertions(+), 109 deletions(-) create mode 100644 src/components/Flex/__stories__/Flex.stories.module.scss delete mode 100644 src/components/Flex/__stories__/Flex.stories.scss create mode 100644 src/storybook/components/related-components/descriptions/list.jsx create mode 100644 src/storybook/components/story-description/story-description.jsx create mode 100644 src/storybook/components/story-description/story-description.module.scss diff --git a/src/components/Flex/Flex.jsx b/src/components/Flex/Flex.jsx index 7fe65ca64b..9b43d2461e 100644 --- a/src/components/Flex/Flex.jsx +++ b/src/components/Flex/Flex.jsx @@ -2,9 +2,9 @@ import React, { useRef, forwardRef } from "react"; import PropTypes from "prop-types"; import cx from "classnames"; import useMergeRefs from "../../hooks/useMergeRefs"; -import { FLEX_POSITIONS } from "./FlexConstants"; -import { BASE_SIZES } from "../../constants/sizes"; +import { FLEX_POSITIONS, FLEX_SPACING_SIZES } from "./FlexConstants"; import classes from "./Flex.module.scss"; +import { BASE_POSITIONS } from "../../constants/positions"; const Flex = forwardRef( ( @@ -17,7 +17,8 @@ const Flex = forwardRef( horizontalPosition, verticalPosition, horizontalSpacingSize, - verticalSpacingSize + verticalSpacingSize, + style }, ref ) => { @@ -26,19 +27,21 @@ const Flex = forwardRef( return (
{children}
@@ -47,9 +50,11 @@ const Flex = forwardRef( ); Flex.horizontalPositions = FLEX_POSITIONS; -Flex.verticalPositions = FLEX_POSITIONS; -Flex.verticalSpacingSizes = BASE_SIZES; -Flex.horizontalSpacingSizes = BASE_SIZES; +Flex.verticalPositions = BASE_POSITIONS; +Flex.verticalSpacingSizes = FLEX_SPACING_SIZES; +Flex.horizontalSpacingSizes = FLEX_SPACING_SIZES; +console.log(Flex); + Flex.propTypes = { /** * class name to be add to the wrapper @@ -59,6 +64,7 @@ Flex.propTypes = { * id to be add to the wrapper */ id: PropTypes.string, + style: PropTypes.object, vertical: PropTypes.bool, wrap: PropTypes.bool, children: PropTypes.element, @@ -76,6 +82,10 @@ Flex.propTypes = { Flex.verticalPositions.SPACE_BETWEEN, Flex.verticalPositions.SPACE_AROUND ]), + horizontalSpacingSize: + PropTypes.oneOf[ + (Flex.verticalSpacingSizes.SMALL, Flex.verticalSpacingSizes.MEDIUM, Flex.verticalSpacingSizes.LARGE) + ], verticalSpacingSize: PropTypes.oneOf[ (Flex.verticalSpacingSizes.SMALL, Flex.verticalSpacingSizes.MEDIUM, Flex.verticalSpacingSizes.LARGE) @@ -86,8 +96,13 @@ Flex.defaultProps = { className: "", id: undefined, vertical: false, + style: undefined, wrap: false, - children: undefined + children: undefined, + horizontalPosition: Flex.horizontalPositions.START, + horizontalSpacingSize: Flex.horizontalSpacingSizes.SMALL, + verticalPosition: Flex.verticalPositions.CENTER, + verticalSpacingSize: Flex.verticalSpacingSizes.NONE }; export default Flex; diff --git a/src/components/Flex/Flex.module.scss b/src/components/Flex/Flex.module.scss index 5a79e0ac5d..4a290e22ff 100644 --- a/src/components/Flex/Flex.module.scss +++ b/src/components/Flex/Flex.module.scss @@ -23,30 +23,22 @@ } @mixin secondary-position-classes { &-start { - align-content: flex-start; + align-items: flex-start; } &-end { - align-content: flex-end; + align-items: flex-end; } &-center { - align-content: center; - } - - &-space-between { - align-content: space-between; - } - - &-space-around { - align-content: space-around; + align-items: center; } } .flex-container { display: flex; flex-direction: row; - & .horizontal { + &.horizontal { &-position { @include main-position-classes; } @@ -68,7 +60,6 @@ } } } - &.vertical { &-flex-container { flex-direction: column; @@ -85,7 +76,24 @@ @include secondary-position-classes; } &-spacing-size { - @include spacing-size-classes(block-end) + &-small { + & > * { + margin-block-end: var(--spacing-small); + } + } + &-medium { + & > * { + margin-block-end: var(--spacing-medium); + } + } + &-large { + & > * { + margin-block-end: var(--spacing-large); + } + } } } + &.wrap { + flex-wrap: wrap; + } } \ No newline at end of file diff --git a/src/components/Flex/FlexConstants.js b/src/components/Flex/FlexConstants.js index 4a1b36ea0b..dffdbd0630 100644 --- a/src/components/Flex/FlexConstants.js +++ b/src/components/Flex/FlexConstants.js @@ -1,7 +1,13 @@ import { BASE_POSITIONS } from "../../constants/positions"; +import { BASE_SIZES } from "../../constants/sizes"; export const FLEX_POSITIONS = Object.freeze({ ...BASE_POSITIONS, SPACE_AROUND: "space-around", SPACE_BETWEEN: "space-between" }); + +export const FLEX_SPACING_SIZES = Object.freeze({ + ...BASE_SIZES, + NONE: "none" +}); diff --git a/src/components/Flex/__stories__/Flex.stories.mdx b/src/components/Flex/__stories__/Flex.stories.mdx index bdf0a7108e..e2ad9d5626 100644 --- a/src/components/Flex/__stories__/Flex.stories.mdx +++ b/src/components/Flex/__stories__/Flex.stories.mdx @@ -1,12 +1,14 @@ import Flex from "../Flex"; import { ArgsTable, Story, Canvas, Meta } from "@storybook/addon-docs"; -import { createComponentTemplate } from "../../../storybook/functions/create-component-story"; -import "./Flex.stories.scss"; -import Icon from "../../Icon/Icon"; -import { Add, Close, Team } from "../../Icon/Icons"; +import { Add, Search, Person, Filter, Sort } from "../../Icon/Icons"; +import Button from "../../Button/Button"; +import classes from "./Flex.stories.module.scss"; +import Chips from "../../Chips/Chips"; +import { StoryDescription } from "../../../storybook/components/story-description/story-description"; +import { LIST, MENU, TABS } from "../../../storybook/components/related-components/component-description-map"; @@ -14,9 +16,9 @@ import { Add, Close, Team } from "../../Icon/Icons"; export const flexTemplate = (args) => { return - - - + + + } @@ -47,98 +49,232 @@ Use Flex component to position group of sub-elements in one dimension, horizonta ## Usage -Tip content - ## Variants -### Story title 1 -Description of story 1 +### Directions - - - + +
+ + + + + + + + + + + + + + +
-### Story title 2 -Description of story 2 +### Horizontal spacing between items - - - + +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + +
-## Do’s and Don’ts -, - description:"Best practice description." - }, - negative: { - component:, - description:"Bad practice description." - } - }, - { - positive: { - component:, - description:"Best practice description." - }, - negative: { - component:, - description:"Bad practice description." - } - }, - { - positive: { - component:, - description:"Best practice description." - }, - negative: { - component:, - description:"Bad practice description." - } - }, - { - positive: { - component:, - description:"Best practice description." - }, - negative: { - component:, - description:"Bad practice description." - } - } - ]} -/> +### Vertical spacing between items + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +### Horizontal positions + + +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+
+
+### Vertical positions + + + + + + + + + + + + + + + + + + + + + + + + + + -## Use cases and examples -### Story title 3 -Description of story 3 +### Support multi lines layout +You can display a layout that includes multiple lines using the flex component wrap mode. +This mode allows the layout to break into multiple lines if all the component children cannot fit into one only. - - - + + + + + + + + + + -### Story title 4 -Description of story 4 +## Use cases and examples + +### Flex as toolbar container +You can use flex component for create responsive toolbars - - - + + + + + + + + + ## Related components - \ No newline at end of file + \ No newline at end of file diff --git a/src/components/Flex/__stories__/Flex.stories.module.scss b/src/components/Flex/__stories__/Flex.stories.module.scss new file mode 100644 index 0000000000..742849dac0 --- /dev/null +++ b/src/components/Flex/__stories__/Flex.stories.module.scss @@ -0,0 +1,11 @@ +@import "../../../styles/themes.scss"; + +.multi-lines-story-search { + width: 200px; +} + +.story-container { + & > * { + margin-bottom: var(--spacing-large); + } +} \ No newline at end of file diff --git a/src/components/Flex/__stories__/Flex.stories.scss b/src/components/Flex/__stories__/Flex.stories.scss deleted file mode 100644 index cfecda8b15..0000000000 --- a/src/components/Flex/__stories__/Flex.stories.scss +++ /dev/null @@ -1,5 +0,0 @@ -@import "../../../styles/themes.scss"; - -.monday-storybook-flex { - -} diff --git a/src/storybook/components/related-components/component-description-map.js b/src/storybook/components/related-components/component-description-map.js index 24a227890d..d0bffb8a78 100644 --- a/src/storybook/components/related-components/component-description-map.js +++ b/src/storybook/components/related-components/component-description-map.js @@ -36,6 +36,7 @@ import { IconButtonDescription } from "./descriptions/icon-button-description"; import { MenuButtonDescription } from "./descriptions/menu-button-description"; import { ClickableDescription } from "./descriptions/clickable-description/clickable-description"; import { HiddenTextDescription } from "./descriptions/hidden-text-description"; +import { ListDescription } from "./descriptions/list"; export const SPLIT_BUTTON = "split-button"; export const BUTTON_GROUP = "button-group"; @@ -71,6 +72,7 @@ export const ICON_BUTTON = "icon-button"; export const MENU_BUTTON = "menu-button"; export const CLICKABLE = "clickable"; export const HIDDEN_TEXT = "hidden-text-description"; +export const LIST = "list"; // General description names (not related to specific components) export const COLORS = "colors"; @@ -114,6 +116,7 @@ descriptionTypesMap.set(EDITABLE_HEADING, ); descriptionTypesMap.set(HEADING, ); descriptionTypesMap.set(CLICKABLE, ); descriptionTypesMap.set(HIDDEN_TEXT, ); +descriptionTypesMap.set(LIST, ); // General description (not related to specific components) descriptionTypesMap.set(COLORS, ); diff --git a/src/storybook/components/related-components/descriptions/list.jsx b/src/storybook/components/related-components/descriptions/list.jsx new file mode 100644 index 0000000000..16bf01aeb0 --- /dev/null +++ b/src/storybook/components/related-components/descriptions/list.jsx @@ -0,0 +1,29 @@ +import { useMemo } from "react"; +import { RelatedComponent } from "../../related-component/related-component"; +import DialogContentContainer from "../../../../components/DialogContentContainer/DialogContentContainer"; +import ListItem from "../../../../components/ListItem/ListItem"; +import List from "../../../../components/List/List"; + +export const ListDescription = () => { + const component = useMemo(() => { + return ( +
+ + + List item 1 + List item 2 + List item 3 + + +
+ ); + }, []); + return ( + + ); +}; diff --git a/src/storybook/components/story-description/story-description.jsx b/src/storybook/components/story-description/story-description.jsx new file mode 100644 index 0000000000..fe5e643117 --- /dev/null +++ b/src/storybook/components/story-description/story-description.jsx @@ -0,0 +1,34 @@ +import PropTypes from "prop-types"; +import Flex from "../../../components/Flex/Flex"; +import classes from "./story-description.module.scss"; + +export const StoryDescription = ({ description, children, vertical }) => ( + + + {description} + + {children} + +); + +StoryDescription.propTypes = { + description: PropTypes.string, + children: PropTypes.element, + vertical: PropTypes.bool +}; + +StoryDescription.defaultProps = { + description: "", + children: null, + vertical: false +}; diff --git a/src/storybook/components/story-description/story-description.module.scss b/src/storybook/components/story-description/story-description.module.scss new file mode 100644 index 0000000000..dfb6b85a6b --- /dev/null +++ b/src/storybook/components/story-description/story-description.module.scss @@ -0,0 +1,4 @@ +.description { + font-weight: 500; + text-align: center; +} \ No newline at end of file From 8f49677d13051edd95fa93a5ee0cb4598889573d Mon Sep 17 00:00:00 2001 From: Hadas Farhi Date: Thu, 13 Jan 2022 15:36:16 +0200 Subject: [PATCH 03/11] flex component --- src/components/Flex/Flex.jsx | 23 +- .../flex-snapshot-tests.jest.js.snap | 199 ++++++++++++++++++ .../__tests__/flex-snapshot-tests.jest.js | 188 +++++++++++++---- .../Flex/__tests__/flex-tests.jest.js | 19 -- 4 files changed, 360 insertions(+), 69 deletions(-) create mode 100644 src/components/Flex/__tests__/__snapshots__/flex-snapshot-tests.jest.js.snap delete mode 100644 src/components/Flex/__tests__/flex-tests.jest.js diff --git a/src/components/Flex/Flex.jsx b/src/components/Flex/Flex.jsx index 9b43d2461e..a32574d4e6 100644 --- a/src/components/Flex/Flex.jsx +++ b/src/components/Flex/Flex.jsx @@ -53,7 +53,6 @@ Flex.horizontalPositions = FLEX_POSITIONS; Flex.verticalPositions = BASE_POSITIONS; Flex.verticalSpacingSizes = FLEX_SPACING_SIZES; Flex.horizontalSpacingSizes = FLEX_SPACING_SIZES; -console.log(Flex); Flex.propTypes = { /** @@ -67,7 +66,7 @@ Flex.propTypes = { style: PropTypes.object, vertical: PropTypes.bool, wrap: PropTypes.bool, - children: PropTypes.element, + children: PropTypes.oneOfType([PropTypes.element, PropTypes.arrayOf(PropTypes.element)]), horizontalPosition: PropTypes.oneOf([ Flex.horizontalPositions.START, Flex.horizontalPositions.CENTER, @@ -82,14 +81,18 @@ Flex.propTypes = { Flex.verticalPositions.SPACE_BETWEEN, Flex.verticalPositions.SPACE_AROUND ]), - horizontalSpacingSize: - PropTypes.oneOf[ - (Flex.verticalSpacingSizes.SMALL, Flex.verticalSpacingSizes.MEDIUM, Flex.verticalSpacingSizes.LARGE) - ], - verticalSpacingSize: - PropTypes.oneOf[ - (Flex.verticalSpacingSizes.SMALL, Flex.verticalSpacingSizes.MEDIUM, Flex.verticalSpacingSizes.LARGE) - ] + horizontalSpacingSize: PropTypes.oneOf([ + Flex.horizontalSpacingSizes.NONE, + Flex.horizontalSpacingSizes.SMALL, + Flex.horizontalSpacingSizes.MEDIUM, + Flex.horizontalSpacingSizes.LARGE + ]), + verticalSpacingSize: PropTypes.oneOf([ + Flex.verticalSpacingSizes.NONE, + Flex.verticalSpacingSizes.SMALL, + Flex.verticalSpacingSizes.MEDIUM, + Flex.verticalSpacingSizes.LARGE + ]) }; Flex.defaultProps = { diff --git a/src/components/Flex/__tests__/__snapshots__/flex-snapshot-tests.jest.js.snap b/src/components/Flex/__tests__/__snapshots__/flex-snapshot-tests.jest.js.snap new file mode 100644 index 0000000000..75121df2f2 --- /dev/null +++ b/src/components/Flex/__tests__/__snapshots__/flex-snapshot-tests.jest.js.snap @@ -0,0 +1,199 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Flex renders correctly Horizontal display with children 1`] = ` +
+
+ 1 +
+
+ 2 +
+
+ 3 +
+
+`; + +exports[`Flex renders correctly Horizontal display with horizontal position 1`] = ` +
+
+ 1 +
+
+ 2 +
+
+ 3 +
+
+`; + +exports[`Flex renders correctly Horizontal display with horizontal spacing 1`] = ` +
+
+ 1 +
+
+ 2 +
+
+ 3 +
+
+`; + +exports[`Flex renders correctly Horizontal display with vertical position 1`] = ` +
+
+ 1 +
+
+ 2 +
+
+ 3 +
+
+`; + +exports[`Flex renders correctly Horizontal display with vertical spacing 1`] = ` +
+
+ 1 +
+
+ 2 +
+
+ 3 +
+
+`; + +exports[`Flex renders correctly Horizontal display with wrap 1`] = ` +
+
+ 1 +
+
+ 2 +
+
+ 3 +
+
+`; + +exports[`Flex renders correctly Vertical display with children 1`] = ` +
+
+ 1 +
+
+ 2 +
+
+ 3 +
+
+`; + +exports[`Flex renders correctly Vertical display with horizontal position 1`] = ` +
+
+ 1 +
+
+ 2 +
+
+ 3 +
+
+`; + +exports[`Flex renders correctly Vertical display with horizontal spacing 1`] = ` +
+
+ 1 +
+
+ 2 +
+
+ 3 +
+
+`; + +exports[`Flex renders correctly Vertical display with vertical position 1`] = ` +
+
+ 1 +
+
+ 2 +
+
+ 3 +
+
+`; + +exports[`Flex renders correctly Vertical display with vertical spacing 1`] = ` +
+
+ 1 +
+
+ 2 +
+
+ 3 +
+
+`; + +exports[`Flex renders correctly Vertical display with wrap 1`] = ` +
+
+ 1 +
+
+ 2 +
+
+ 3 +
+
+`; + +exports[`Flex renders correctly with empty props 1`] = ` +
+`; diff --git a/src/components/Flex/__tests__/flex-snapshot-tests.jest.js b/src/components/Flex/__tests__/flex-snapshot-tests.jest.js index 5846db0f4b..583e624c85 100644 --- a/src/components/Flex/__tests__/flex-snapshot-tests.jest.js +++ b/src/components/Flex/__tests__/flex-snapshot-tests.jest.js @@ -2,49 +2,157 @@ import React from "react"; import renderer from "react-test-renderer"; import Flex from "../Flex"; -/** - * There are cases where the component we want to test in the snapshot test will contain additional components. - We do not want changes to the additional components to fail our component snapshot's test. - Therefore, we will replace the instances of the other external components in the snapshot test with mock/stub components in these cases. - */ - -/** example for external library - jest.mock("react-transition-group", () => { - const FakeTransition = jest.fn(({ children }) => children); - const FakeSwitchTransition = jest.fn(({ children }) => children); - const FakeCSSTransition = jest.fn(({ children }) => children); - - // We return here the instance of the mock / stub library object content - return { - CSSTransition: FakeCSSTransition, - Transition: FakeTransition, - SwitchTransition: FakeSwitchTransition - }; -}); - **/ - -/** example for internal component -jest.mock("../../Button/Button", () => { - // We return here the instance of the mock / stub component - return ({ onClick }) => ( -
- ); -}); -**/ - describe("Flex renders correctly", () => { - it("with empty props", () => { - const tree = renderer.create().toJSON(); + it("with empty props", () => { + const tree = renderer.create().toJSON(); + expect(tree).toMatchSnapshot(); + }); + describe("Horizontal display", () => { + it("with children", () => { + const tree = renderer + .create( + +
1
+
2
+
3
+
+ ) + .toJSON(); expect(tree).toMatchSnapshot(); }); - - it("with x", () => { - const tree = renderer.create().toJSON(); + it("with horizontal position", () => { + const tree = renderer + .create( + +
1
+
2
+
3
+
+ ) + .toJSON(); expect(tree).toMatchSnapshot(); }); - - it("with y", () => { - const tree = renderer.create().toJSON(); - expect(tree).toMatchSnapshot(); + it("with horizontal spacing", () => { + const tree = renderer + .create( + +
1
+
2
+
3
+
+ ) + .toJSON(); + expect(tree).toMatchSnapshot(); + }); + it("with vertical position", () => { + const tree = renderer + .create( + +
1
+
2
+
3
+
+ ) + .toJSON(); + expect(tree).toMatchSnapshot(); + }); + it("with vertical spacing", () => { + const tree = renderer + .create( + +
1
+
2
+
3
+
+ ) + .toJSON(); + expect(tree).toMatchSnapshot(); + }); + it("with wrap", () => { + const tree = renderer + .create( + +
1
+
2
+
3
+
+ ) + .toJSON(); + expect(tree).toMatchSnapshot(); + }); + }); + describe("Vertical display", () => { + it("with children", () => { + const tree = renderer + .create( + +
1
+
2
+
3
+
+ ) + .toJSON(); + expect(tree).toMatchSnapshot(); + }); + it("with horizontal position", () => { + const tree = renderer + .create( + +
1
+
2
+
3
+
+ ) + .toJSON(); + expect(tree).toMatchSnapshot(); + }); + it("with horizontal spacing", () => { + const tree = renderer + .create( + +
1
+
2
+
3
+
+ ) + .toJSON(); + expect(tree).toMatchSnapshot(); }); -}); \ No newline at end of file + it("with vertical position", () => { + const tree = renderer + .create( + +
1
+
2
+
3
+
+ ) + .toJSON(); + expect(tree).toMatchSnapshot(); + }); + it("with vertical spacing", () => { + const tree = renderer + .create( + +
1
+
2
+
3
+
+ ) + .toJSON(); + expect(tree).toMatchSnapshot(); + }); + it("with wrap", () => { + const tree = renderer + .create( + +
1
+
2
+
3
+
+ ) + .toJSON(); + expect(tree).toMatchSnapshot(); + }); + }); +}); diff --git a/src/components/Flex/__tests__/flex-tests.jest.js b/src/components/Flex/__tests__/flex-tests.jest.js deleted file mode 100644 index da0d50b107..0000000000 --- a/src/components/Flex/__tests__/flex-tests.jest.js +++ /dev/null @@ -1,19 +0,0 @@ -import React from "react"; -import { fireEvent, render, cleanup } from "@testing-library/react"; -import { act } from "@testing-library/react-hooks"; -import Flex from "../Flex"; - - -const renderComponent = props => { - return render(); -}; - -describe("Flex tests", () => { - afterEach(() => { - cleanup(); - }); - - it("calls x when y", () => {}); - - it("should do x when y", () => {}); -}); From b463e6e4501a1b42d3b0472298f70d8fd27d3da5 Mon Sep 17 00:00:00 2001 From: Hadas Farhi Date: Thu, 13 Jan 2022 16:10:48 +0200 Subject: [PATCH 04/11] flex component --- src/components/Flex/Flex.jsx | 14 ++--- src/components/Flex/Flex.module.scss | 52 +++++++++---------- src/components/Flex/FlexConstants.js | 10 ++-- src/constants/positions.js | 6 +-- src/constants/sizes.js | 2 + .../story-description/story-description.jsx | 2 +- 6 files changed, 42 insertions(+), 44 deletions(-) diff --git a/src/components/Flex/Flex.jsx b/src/components/Flex/Flex.jsx index a32574d4e6..7e52a6bba4 100644 --- a/src/components/Flex/Flex.jsx +++ b/src/components/Flex/Flex.jsx @@ -30,15 +30,15 @@ const Flex = forwardRef( id={id} ref={mergedRef} className={cx( - classes["flex-container"], - classes[`horizontal-position-${horizontalPosition}`], - classes[`horizontal-spacing-size-${horizontalSpacingSize}`], - classes[`vertical-position-${verticalPosition}`], - classes[`vertical-spacing-size-${verticalSpacingSize}`], + classes.container, + classes[`horizontalPosition${horizontalPosition}`], + classes[`horizontalSpacingSize${horizontalSpacingSize}`], + classes[`verticalPosition${verticalPosition}`], + classes[`verticalSpacingSize${verticalSpacingSize}`], className, { - [classes["vertical-flex-container"]]: vertical, - [classes["wrap"]]: wrap + [classes.vertical]: vertical, + [classes.wrap]: wrap } )} style={style} diff --git a/src/components/Flex/Flex.module.scss b/src/components/Flex/Flex.module.scss index 4a290e22ff..4b272696b9 100644 --- a/src/components/Flex/Flex.module.scss +++ b/src/components/Flex/Flex.module.scss @@ -1,59 +1,59 @@ @import "../../styles/themes.scss"; @mixin main-position-classes { - &-start { + &Start { justify-content: flex-start; } - &-end { + &End { justify-content: flex-end; } - &-center { + &Center { justify-content: center; } - &-space-between { + &SpaceBetween { justify-content: space-between; } - &-space-around { + &SpaceAround { justify-content: space-around; } } @mixin secondary-position-classes { - &-start { + &Start { align-items: flex-start; } - &-end { + &End { align-items: flex-end; } - &-center { + &Center { align-items: center; } } -.flex-container { +.container { display: flex; flex-direction: row; &.horizontal { - &-position { + &Position { @include main-position-classes; } - &-spacing-size { - &-small { + &SpacingSize { + &Small { & > * { margin-inline-end: var(--spacing-small); } } - &-medium { + &Medium { & > * { margin-inline-end: var(--spacing-medium); } } - &-large { + &Large { & > * { margin-inline-end: var(--spacing-large); } @@ -61,32 +61,28 @@ } } &.vertical { - &-flex-container { - flex-direction: column; + flex-direction: column; - & .vertical-position { - @include main-position-classes; - } - - & .horizontal-position { - @include secondary-position-classes; - } + & .verticalPosition { + @include main-position-classes; } - &-position { + + & .horizontalPosition { @include secondary-position-classes; } - &-spacing-size { - &-small { + + &SpacingSize { + &Small { & > * { margin-block-end: var(--spacing-small); } } - &-medium { + &Medium { & > * { margin-block-end: var(--spacing-medium); } } - &-large { + &Large { & > * { margin-block-end: var(--spacing-large); } diff --git a/src/components/Flex/FlexConstants.js b/src/components/Flex/FlexConstants.js index dffdbd0630..8d66469ee2 100644 --- a/src/components/Flex/FlexConstants.js +++ b/src/components/Flex/FlexConstants.js @@ -1,13 +1,13 @@ import { BASE_POSITIONS } from "../../constants/positions"; -import { BASE_SIZES } from "../../constants/sizes"; +import { PASCAL_BASE_SIZE } from "../../constants/sizes"; export const FLEX_POSITIONS = Object.freeze({ ...BASE_POSITIONS, - SPACE_AROUND: "space-around", - SPACE_BETWEEN: "space-between" + SPACE_AROUND: "SpaceAround", + SPACE_BETWEEN: "SpaceBetween" }); export const FLEX_SPACING_SIZES = Object.freeze({ - ...BASE_SIZES, - NONE: "none" + ...PASCAL_BASE_SIZE, + NONE: "None" }); diff --git a/src/constants/positions.js b/src/constants/positions.js index b73880a2ba..210b2dd4be 100644 --- a/src/constants/positions.js +++ b/src/constants/positions.js @@ -1,5 +1,5 @@ export const BASE_POSITIONS = { - START: "start", - CENTER: "center", - END: "end" + START: "Start", + CENTER: "Center", + END: "End" }; diff --git a/src/constants/sizes.js b/src/constants/sizes.js index b71240a0b8..92a5f3668e 100644 --- a/src/constants/sizes.js +++ b/src/constants/sizes.js @@ -1,3 +1,5 @@ +export const PASCAL_BASE_SIZE = Object.freeze({ SMALL: "Small", MEDIUM: "Medium", LARGE: "Large" }); + export const BASE_SIZES = Object.freeze({ SMALL: "small", MEDIUM: "medium", LARGE: "large" }); export const SIZES = Object.freeze({ diff --git a/src/storybook/components/story-description/story-description.jsx b/src/storybook/components/story-description/story-description.jsx index fe5e643117..127e76eeea 100644 --- a/src/storybook/components/story-description/story-description.jsx +++ b/src/storybook/components/story-description/story-description.jsx @@ -10,7 +10,7 @@ export const StoryDescription = ({ description, children, vertical }) => ( verticalPosition={Flex.verticalPositions.CENTER} > Date: Mon, 17 Jan 2022 09:38:09 +0200 Subject: [PATCH 05/11] flex component --- src/components/Flex/__stories__/Flex.stories.mdx | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/components/Flex/__stories__/Flex.stories.mdx b/src/components/Flex/__stories__/Flex.stories.mdx index e2ad9d5626..00ee7f871e 100644 --- a/src/components/Flex/__stories__/Flex.stories.mdx +++ b/src/components/Flex/__stories__/Flex.stories.mdx @@ -216,22 +216,25 @@ Use Flex component to position group of sub-elements in one dimension, horizonta ### Vertical positions - + - + - + - - + + From baa4d71c1c57353d66c02ec3ba5f32ea39697aaf Mon Sep 17 00:00:00 2001 From: Hadas Farhi Date: Mon, 17 Jan 2022 09:59:02 +0200 Subject: [PATCH 06/11] try to fix order stories problem --- .storybook/preview.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.storybook/preview.js b/.storybook/preview.js index c807e83697..8c491b6775 100644 --- a/.storybook/preview.js +++ b/.storybook/preview.js @@ -74,7 +74,11 @@ addParameters({ "*", "Accessibility", "Hooks" - ] + ], + includeName: false, + // currently there is a bug that makes any componet stories to be order alphabetical even so + // it is not the default settings. This settings purpose is to sort all the stories of any component by their load time + storySort: (a, b) => 0 } } }); From 7becd359daf588bade3de6e2e61033139eb35caa Mon Sep 17 00:00:00 2001 From: Hadas Farhi Date: Mon, 17 Jan 2022 12:00:28 +0200 Subject: [PATCH 07/11] try to fix order stories problem --- src/components/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/index.js b/src/components/index.js index 5ca4620fbc..128149edfb 100644 --- a/src/components/index.js +++ b/src/components/index.js @@ -73,4 +73,4 @@ export { default as AccordionItem } from "./Accordion/AccordionItem/AccordionIte export { default as Clickable } from "./Clickable/Clickable"; export { default as ColorUtils } from "../utils/colors-utils"; export { default as IconButton } from "./IconButton/IconButton"; -export { default as Flex } from "./Flex/Flex"; \ No newline at end of file +export { default as Flex } from "./Flex/Flex"; From 75975f19efdbf7bcc8f08ce859ce22bdf3ba017b Mon Sep 17 00:00:00 2001 From: Hadas Farhi Date: Mon, 17 Jan 2022 18:01:20 +0200 Subject: [PATCH 08/11] fix code review notes --- src/components/Flex/Flex.jsx | 98 +++++-------- src/components/Flex/Flex.module.scss | 107 ++++---------- src/components/Flex/FlexConstants.js | 14 +- .../Flex/__stories__/Flex.stories.mdx | 132 ++++++++++-------- .../Flex/__stories__/Flex.stories.module.scss | 4 + .../story-description/story-description.jsx | 34 ++--- .../story-description.module.scss | 5 +- 7 files changed, 177 insertions(+), 217 deletions(-) diff --git a/src/components/Flex/Flex.jsx b/src/components/Flex/Flex.jsx index 7e52a6bba4..aad2074345 100644 --- a/src/components/Flex/Flex.jsx +++ b/src/components/Flex/Flex.jsx @@ -1,58 +1,47 @@ -import React, { useRef, forwardRef } from "react"; +import React, { useRef, forwardRef, useMemo } from "react"; import PropTypes from "prop-types"; import cx from "classnames"; import useMergeRefs from "../../hooks/useMergeRefs"; -import { FLEX_POSITIONS, FLEX_SPACING_SIZES } from "./FlexConstants"; -import classes from "./Flex.module.scss"; +import { FLEX_POSITIONS, FLEX_GAPS, FLEX_DIRECTIONS } from "./FlexConstants"; import { BASE_POSITIONS } from "../../constants/positions"; - +import Clickable from "../Clickable/Clickable"; +import classes from "./Flex.module.scss"; const Flex = forwardRef( - ( - { - className, - id, - vertical, - wrap, - children, - horizontalPosition, - verticalPosition, - horizontalSpacingSize, - verticalSpacingSize, - style - }, - ref - ) => { + ({ className, id, elementType, direction, wrap, children, justify, align, gap, onClick, style }, ref) => { const componentRef = useRef(null); const mergedRef = useMergeRefs({ refs: [ref, componentRef] }); + const overrideStyle = useMemo(() => ({ ...style, gap: `${gap}px` }), [style, gap]); + const Element = onClick ? Clickable : elementType; return ( -
{children} -
+ ); } ); -Flex.horizontalPositions = FLEX_POSITIONS; -Flex.verticalPositions = BASE_POSITIONS; -Flex.verticalSpacingSizes = FLEX_SPACING_SIZES; -Flex.horizontalSpacingSizes = FLEX_SPACING_SIZES; +Flex.justify = FLEX_POSITIONS; +Flex.align = BASE_POSITIONS; +Flex.gaps = FLEX_GAPS; +Flex.directions = FLEX_DIRECTIONS; Flex.propTypes = { /** @@ -64,48 +53,35 @@ Flex.propTypes = { */ id: PropTypes.string, style: PropTypes.object, - vertical: PropTypes.bool, + direction: PropTypes.oneOf([Flex.directions.ROW, Flex.directions.COLUMN]), + elementType: PropTypes.string, wrap: PropTypes.bool, children: PropTypes.oneOfType([PropTypes.element, PropTypes.arrayOf(PropTypes.element)]), - horizontalPosition: PropTypes.oneOf([ - Flex.horizontalPositions.START, - Flex.horizontalPositions.CENTER, - Flex.horizontalPositions.END, - Flex.horizontalPositions.SPACE_BETWEEN, - Flex.horizontalPositions.SPACE_AROUND - ]), - verticalPosition: PropTypes.oneOf([ - Flex.verticalPositions.START, - Flex.verticalPositions.CENTER, - Flex.verticalPositions.END, - Flex.verticalPositions.SPACE_BETWEEN, - Flex.verticalPositions.SPACE_AROUND - ]), - horizontalSpacingSize: PropTypes.oneOf([ - Flex.horizontalSpacingSizes.NONE, - Flex.horizontalSpacingSizes.SMALL, - Flex.horizontalSpacingSizes.MEDIUM, - Flex.horizontalSpacingSizes.LARGE + justify: PropTypes.oneOf([ + Flex.justify.START, + Flex.justify.CENTER, + Flex.justify.END, + Flex.justify.SPACE_BETWEEN, + Flex.justify.SPACE_AROUND ]), - verticalSpacingSize: PropTypes.oneOf([ - Flex.verticalSpacingSizes.NONE, - Flex.verticalSpacingSizes.SMALL, - Flex.verticalSpacingSizes.MEDIUM, - Flex.verticalSpacingSizes.LARGE + align: PropTypes.oneOf([Flex.align.START, Flex.align.CENTER, Flex.align.END]), + gap: PropTypes.oneOfType([ + PropTypes.oneOf([Flex.gaps.NONE, Flex.gaps.SMALL, Flex.gaps.MEDIUM, Flex.gaps.LARGE]), + PropTypes.number ]) }; Flex.defaultProps = { className: "", id: undefined, - vertical: false, + elementType: "div", style: undefined, wrap: false, children: undefined, - horizontalPosition: Flex.horizontalPositions.START, - horizontalSpacingSize: Flex.horizontalSpacingSizes.SMALL, - verticalPosition: Flex.verticalPositions.CENTER, - verticalSpacingSize: Flex.verticalSpacingSizes.NONE + direction: Flex.directions.ROW, + justify: Flex.justify.START, + align: Flex.align.CENTER, + gap: Flex.gaps.NONE }; export default Flex; diff --git a/src/components/Flex/Flex.module.scss b/src/components/Flex/Flex.module.scss index 4b272696b9..4b774dfe88 100644 --- a/src/components/Flex/Flex.module.scss +++ b/src/components/Flex/Flex.module.scss @@ -1,92 +1,45 @@ @import "../../styles/themes.scss"; -@mixin main-position-classes { - &Start { - justify-content: flex-start; - } - - &End { - justify-content: flex-end; - } - - &Center { - justify-content: center; - } - - &SpaceBetween { - justify-content: space-between; - } - - &SpaceAround { - justify-content: space-around; - } -} -@mixin secondary-position-classes { - &Start { - align-items: flex-start; - } - - &End { - align-items: flex-end; - } - - &Center { - align-items: center; - } -} - .container { display: flex; flex-direction: row; - &.horizontal { - &Position { - @include main-position-classes; + &.justify { + &Start { + justify-content: flex-start; + } + + &End { + justify-content: flex-end; } - &SpacingSize { - &Small { - & > * { - margin-inline-end: var(--spacing-small); - } - } - &Medium { - & > * { - margin-inline-end: var(--spacing-medium); - } - } - &Large { - & > * { - margin-inline-end: var(--spacing-large); - } - } + + &Center { + justify-content: center; } - } - &.vertical { - flex-direction: column; - & .verticalPosition { - @include main-position-classes; + &SpaceBetween { + justify-content: space-between; } - & .horizontalPosition { - @include secondary-position-classes; + &SpaceAround { + justify-content: space-around; + } + } + &.align { + &Start { + align-items: flex-start; + } + + &End { + align-items: flex-end; } - &SpacingSize { - &Small { - & > * { - margin-block-end: var(--spacing-small); - } - } - &Medium { - & > * { - margin-block-end: var(--spacing-medium); - } - } - &Large { - & > * { - margin-block-end: var(--spacing-large); - } - } + &Center { + align-items: center; + } + } + &.direction { + &Column { + flex-direction: column; } } &.wrap { diff --git a/src/components/Flex/FlexConstants.js b/src/components/Flex/FlexConstants.js index 8d66469ee2..b4234970d3 100644 --- a/src/components/Flex/FlexConstants.js +++ b/src/components/Flex/FlexConstants.js @@ -7,7 +7,15 @@ export const FLEX_POSITIONS = Object.freeze({ SPACE_BETWEEN: "SpaceBetween" }); -export const FLEX_SPACING_SIZES = Object.freeze({ - ...PASCAL_BASE_SIZE, - NONE: "None" +export const FLEX_GAPS = Object.freeze({ + XS: 4, + SMALL: 8, + MEDIUM: 16, + LARGE: 24, + NONE: 0 +}); + +export const FLEX_DIRECTIONS = Object.freeze({ + ROW: "Row", + COLUMN: "Column" }); diff --git a/src/components/Flex/__stories__/Flex.stories.mdx b/src/components/Flex/__stories__/Flex.stories.mdx index 00ee7f871e..e43c12dfdb 100644 --- a/src/components/Flex/__stories__/Flex.stories.mdx +++ b/src/components/Flex/__stories__/Flex.stories.mdx @@ -67,7 +67,7 @@ Use Flex component to position group of sub-elements in one dimension, horizonta
- + @@ -82,27 +82,41 @@ Use Flex component to position group of sub-elements in one dimension, horizonta
- + + + + + + + + - + - + - + + + + + + + + @@ -115,53 +129,43 @@ Use Flex component to position group of sub-elements in one dimension, horizonta ### Vertical spacing between items - - - + + + - - + + + + + + + + + - - + + - - + + + + + + + + + @@ -176,34 +180,34 @@ Use Flex component to position group of sub-elements in one dimension, horizonta
- + - + - - + + - + - + @@ -218,23 +222,36 @@ Use Flex component to position group of sub-elements in one dimension, horizonta + justify={Flex.justify.SPACE_AROUND}> - + - + - + + + + + + + + + + + + + + + @@ -244,20 +261,19 @@ Use Flex component to position group of sub-elements in one dimension, horizonta - ### Support multi lines layout You can display a layout that includes multiple lines using the flex component wrap mode. This mode allows the layout to break into multiple lines if all the component children cannot fit into one only. - - - - - - - - + + + + + + + + diff --git a/src/components/Flex/__stories__/Flex.stories.module.scss b/src/components/Flex/__stories__/Flex.stories.module.scss index 742849dac0..4d14119536 100644 --- a/src/components/Flex/__stories__/Flex.stories.module.scss +++ b/src/components/Flex/__stories__/Flex.stories.module.scss @@ -4,6 +4,10 @@ width: 200px; } +.flex-chip { + margin: 0; +} + .story-container { & > * { margin-bottom: var(--spacing-large); diff --git a/src/storybook/components/story-description/story-description.jsx b/src/storybook/components/story-description/story-description.jsx index 127e76eeea..bb76d07d96 100644 --- a/src/storybook/components/story-description/story-description.jsx +++ b/src/storybook/components/story-description/story-description.jsx @@ -1,25 +1,25 @@ +import { useMemo } from "react"; +import cx from "classnames"; import PropTypes from "prop-types"; import Flex from "../../../components/Flex/Flex"; import classes from "./story-description.module.scss"; -export const StoryDescription = ({ description, children, vertical }) => ( - - - {description} +export const StoryDescription = ({ description, children, vertical }) => { + const direction = useMemo(() => (vertical ? Flex.directions.COLUMN : Flex.directions.ROW), [vertical]); + return ( + + + {description} + + {children} - {children} - -); + ); +}; StoryDescription.propTypes = { description: PropTypes.string, diff --git a/src/storybook/components/story-description/story-description.module.scss b/src/storybook/components/story-description/story-description.module.scss index dfb6b85a6b..f1646f2334 100644 --- a/src/storybook/components/story-description/story-description.module.scss +++ b/src/storybook/components/story-description/story-description.module.scss @@ -1,4 +1,7 @@ .description { font-weight: 500; - text-align: center; + + &.vertical { + text-align: center; + } } \ No newline at end of file From 79779776e246141b1c8740f171cb9d607663a532 Mon Sep 17 00:00:00 2001 From: Hadas Farhi Date: Tue, 18 Jan 2022 09:58:17 +0200 Subject: [PATCH 09/11] fix snapshot tests --- .../__tests__/flex-snapshot-tests.jest.js | 53 ++++++------------- 1 file changed, 15 insertions(+), 38 deletions(-) diff --git a/src/components/Flex/__tests__/flex-snapshot-tests.jest.js b/src/components/Flex/__tests__/flex-snapshot-tests.jest.js index 583e624c85..d7cb58cde0 100644 --- a/src/components/Flex/__tests__/flex-snapshot-tests.jest.js +++ b/src/components/Flex/__tests__/flex-snapshot-tests.jest.js @@ -20,10 +20,10 @@ describe("Flex renders correctly", () => { .toJSON(); expect(tree).toMatchSnapshot(); }); - it("with horizontal position", () => { + it("with align", () => { const tree = renderer .create( - +
1
2
3
@@ -32,10 +32,10 @@ describe("Flex renders correctly", () => { .toJSON(); expect(tree).toMatchSnapshot(); }); - it("with horizontal spacing", () => { + it("with justify", () => { const tree = renderer .create( - +
1
2
3
@@ -44,10 +44,10 @@ describe("Flex renders correctly", () => { .toJSON(); expect(tree).toMatchSnapshot(); }); - it("with vertical position", () => { + it("with align", () => { const tree = renderer .create( - +
1
2
3
@@ -56,10 +56,10 @@ describe("Flex renders correctly", () => { .toJSON(); expect(tree).toMatchSnapshot(); }); - it("with vertical spacing", () => { + it("with gap", () => { const tree = renderer .create( - +
1
2
3
@@ -85,7 +85,7 @@ describe("Flex renders correctly", () => { it("with children", () => { const tree = renderer .create( - +
1
2
3
@@ -94,34 +94,11 @@ describe("Flex renders correctly", () => { .toJSON(); expect(tree).toMatchSnapshot(); }); - it("with horizontal position", () => { - const tree = renderer - .create( - -
1
-
2
-
3
-
- ) - .toJSON(); - expect(tree).toMatchSnapshot(); - }); - it("with horizontal spacing", () => { - const tree = renderer - .create( - -
1
-
2
-
3
-
- ) - .toJSON(); - expect(tree).toMatchSnapshot(); - }); - it("with vertical position", () => { + + it("with justify", () => { const tree = renderer .create( - +
1
2
3
@@ -130,10 +107,10 @@ describe("Flex renders correctly", () => { .toJSON(); expect(tree).toMatchSnapshot(); }); - it("with vertical spacing", () => { + it("with align", () => { const tree = renderer .create( - +
1
2
3
@@ -145,7 +122,7 @@ describe("Flex renders correctly", () => { it("with wrap", () => { const tree = renderer .create( - +
1
2
3
From 03e7ee5f7506f4f6bdab40bc4bfca1913200af5a Mon Sep 17 00:00:00 2001 From: Hadas Farhi Date: Tue, 18 Jan 2022 10:11:15 +0200 Subject: [PATCH 10/11] try to fix order stories problem --- .../flex-snapshot-tests.jest.js.snap | 114 ++++++++++++------ 1 file changed, 74 insertions(+), 40 deletions(-) diff --git a/src/components/Flex/__tests__/__snapshots__/flex-snapshot-tests.jest.js.snap b/src/components/Flex/__tests__/__snapshots__/flex-snapshot-tests.jest.js.snap index 75121df2f2..4abd4b8f24 100644 --- a/src/components/Flex/__tests__/__snapshots__/flex-snapshot-tests.jest.js.snap +++ b/src/components/Flex/__tests__/__snapshots__/flex-snapshot-tests.jest.js.snap @@ -1,8 +1,14 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Flex renders correctly Horizontal display with children 1`] = ` +exports[`Flex renders correctly Horizontal display with align 1`] = `
1 @@ -16,9 +22,15 @@ exports[`Flex renders correctly Horizontal display with children 1`] = `
`; -exports[`Flex renders correctly Horizontal display with horizontal position 1`] = ` +exports[`Flex renders correctly Horizontal display with align 2`] = `
1 @@ -32,9 +44,15 @@ exports[`Flex renders correctly Horizontal display with horizontal position 1`]
`; -exports[`Flex renders correctly Horizontal display with horizontal spacing 1`] = ` +exports[`Flex renders correctly Horizontal display with children 1`] = `
1 @@ -48,9 +66,15 @@ exports[`Flex renders correctly Horizontal display with horizontal spacing 1`] =
`; -exports[`Flex renders correctly Horizontal display with vertical position 1`] = ` +exports[`Flex renders correctly Horizontal display with gap 1`] = `
1 @@ -64,9 +88,15 @@ exports[`Flex renders correctly Horizontal display with vertical position 1`] =
`; -exports[`Flex renders correctly Horizontal display with vertical spacing 1`] = ` +exports[`Flex renders correctly Horizontal display with justify 1`] = `
1 @@ -83,6 +113,12 @@ exports[`Flex renders correctly Horizontal display with vertical spacing 1`] = ` exports[`Flex renders correctly Horizontal display with wrap 1`] = `
1 @@ -96,25 +132,15 @@ exports[`Flex renders correctly Horizontal display with wrap 1`] = `
`; -exports[`Flex renders correctly Vertical display with children 1`] = ` -
-
- 1 -
-
- 2 -
-
- 3 -
-
-`; - -exports[`Flex renders correctly Vertical display with horizontal position 1`] = ` +exports[`Flex renders correctly Vertical display with align 1`] = `
1 @@ -128,25 +154,15 @@ exports[`Flex renders correctly Vertical display with horizontal position 1`] =
`; -exports[`Flex renders correctly Vertical display with horizontal spacing 1`] = ` -
-
- 1 -
-
- 2 -
-
- 3 -
-
-`; - -exports[`Flex renders correctly Vertical display with vertical position 1`] = ` +exports[`Flex renders correctly Vertical display with children 1`] = `
1 @@ -160,9 +176,15 @@ exports[`Flex renders correctly Vertical display with vertical position 1`] = `
`; -exports[`Flex renders correctly Vertical display with vertical spacing 1`] = ` +exports[`Flex renders correctly Vertical display with justify 1`] = `
1 @@ -179,6 +201,12 @@ exports[`Flex renders correctly Vertical display with vertical spacing 1`] = ` exports[`Flex renders correctly Vertical display with wrap 1`] = `
1 @@ -195,5 +223,11 @@ exports[`Flex renders correctly Vertical display with wrap 1`] = ` exports[`Flex renders correctly with empty props 1`] = `
`; From 79c1d4dd86946fd574b94f6287c7647cb1c04ef9 Mon Sep 17 00:00:00 2001 From: Hadas Farhi Date: Tue, 18 Jan 2022 10:24:06 +0200 Subject: [PATCH 11/11] fix linter --- src/components/Flex/Flex.jsx | 1 + src/components/Flex/FlexConstants.js | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Flex/Flex.jsx b/src/components/Flex/Flex.jsx index aad2074345..5187ffacdf 100644 --- a/src/components/Flex/Flex.jsx +++ b/src/components/Flex/Flex.jsx @@ -6,6 +6,7 @@ import { FLEX_POSITIONS, FLEX_GAPS, FLEX_DIRECTIONS } from "./FlexConstants"; import { BASE_POSITIONS } from "../../constants/positions"; import Clickable from "../Clickable/Clickable"; import classes from "./Flex.module.scss"; + const Flex = forwardRef( ({ className, id, elementType, direction, wrap, children, justify, align, gap, onClick, style }, ref) => { const componentRef = useRef(null); diff --git a/src/components/Flex/FlexConstants.js b/src/components/Flex/FlexConstants.js index b4234970d3..f725d6c643 100644 --- a/src/components/Flex/FlexConstants.js +++ b/src/components/Flex/FlexConstants.js @@ -1,5 +1,4 @@ import { BASE_POSITIONS } from "../../constants/positions"; -import { PASCAL_BASE_SIZE } from "../../constants/sizes"; export const FLEX_POSITIONS = Object.freeze({ ...BASE_POSITIONS,