Skip to content

Commit

Permalink
fix(System)!: removing spacing prop in favor of Tailwind classes (#836)
Browse files Browse the repository at this point in the history
  • Loading branch information
aversini authored Dec 29, 2024
1 parent 4fb7406 commit 0586551
Show file tree
Hide file tree
Showing 9 changed files with 21 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ const componentsWithNoSpacingProp = [
"ui-pill",
"ui-spinner",
"ui-svgicon",
"ui-system",
"ui-table",
"ui-textarea",
"ui-textinput",
Expand Down
9 changes: 2 additions & 7 deletions packages/ui-system/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
"type": "module",
"main": "dist/index.js",
"types": "dist/index.d.ts",
"files": [
"dist"
],
"files": ["dist"],
"scripts": {
"build:check": "tsc",
"build:js": "vite build",
Expand All @@ -38,12 +36,9 @@
"react-dom": "^18.3.1 || ^19.0.0"
},
"dependencies": {
"@versini/ui-spacing": "workspace:../ui-spacing",
"@versini/ui-types": "workspace:../ui-types",
"clsx": "2.1.1",
"tailwindcss": "3.4.17"
},
"sideEffects": [
"**/*.css"
]
"sideEffects": ["**/*.css"]
}
19 changes: 6 additions & 13 deletions packages/ui-system/src/components/Flexgrid/Flexgrid.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import clsx from "clsx";

import { getSpacing } from "@versini/ui-spacing";
import React from "react";
import { FLEXGRID_CLASSNAME, FLEXGRID_GAP_RATIO } from "../../common/constants";
import { FlexgridContext } from "./FlexgridContext";
import type { FlexgridProps } from "./FlexgridTypes";
Expand All @@ -18,8 +16,6 @@ export const Flexgrid = ({
alignHorizontal = "normal",
alignVertical = "normal",

spacing,

...otherProps
}: FlexgridProps) => {
const cssRoot = {
Expand All @@ -39,21 +35,18 @@ export const Flexgrid = ({

const flexgridClassName = clsx(
FLEXGRID_CLASSNAME,
className,
"box-border flex flex-wrap",
className,
);

const context = { columnGap, rowGap };
const Component = spacing ? "div" : React.Fragment;

return (
<Component {...(spacing ? { className: getSpacing(spacing) } : {})}>
<div className={flexgridClassName} style={cssRoot} {...otherProps}>
<FlexgridContext.Provider value={context}>
{children}
</FlexgridContext.Provider>
</div>
</Component>
<div className={flexgridClassName} style={cssRoot} {...otherProps}>
<FlexgridContext.Provider value={context}>
{children}
</FlexgridContext.Provider>
</div>
);
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import type { SpacingTypes } from "@versini/ui-types";

export type FlexgridProps = {
/**
* Children of the Flexgrid (FlexgridItem(s) or any other nodes).
Expand Down Expand Up @@ -72,7 +70,7 @@ export type FlexgridProps = {
* It follows the [CSS width property](https://developer.mozilla.org/en-US/docs/Web/CSS/width).
*/
width?: string;
} & SpacingTypes.Props;
};

export type FlexgridItemProps = {
/** Children of the FlexgridItem. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,14 +102,16 @@ describe("Flexgrid props", () => {
expectToHaveStyles(gridRoot, { "align-items": "stretch" });
});

it("should respect the spacing prop", async () => {
it("should respect the spacing via className", async () => {
render(
<Flexgrid alignVertical="stretch" data-testid="grid-1" spacing={20}>
<Flexgrid alignVertical="stretch" data-testid="grid-1" className="mr-2">
hello
</Flexgrid>,
);
const gridRoot = await screen.findByTestId("grid-1");
expectToHaveStyles(gridRoot, { "align-items": "stretch" });
expect(gridRoot.parentElement).toHaveClass("m-20");
// not only it should be there, but it should be the last entry
expect(gridRoot).toHaveClass("mr-2");
expect(gridRoot.className.slice(-4)).toBe("mr-2");
});
});
17 changes: 5 additions & 12 deletions packages/ui-system/src/components/ThemeProvider/ThemeProvider.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import clsx from "clsx";
import React, { useEffect, useRef } from "react";
import { useEffect, useRef } from "react";

import { getSpacing } from "@versini/ui-spacing";
import { THEMEPROVIDER_CLASSNAME } from "../../common/constants";
import { ThemeProviderProps } from "./ThemeProviderTypes";

Expand All @@ -10,11 +9,9 @@ export const ThemeProvider = ({
children,
global,
className,
spacing,
}: ThemeProviderProps) => {
const wrapperRef = useRef<HTMLDivElement>(null);
const wrapperClass = clsx(THEMEPROVIDER_CLASSNAME, "contents", className);
const Component = spacing ? "div" : React.Fragment;

useEffect(() => {
const wrapper = global
Expand All @@ -29,14 +26,10 @@ export const ThemeProvider = ({
}, [customTheme, global]);

return customTheme || !global ? (
<Component {...(spacing ? { className: getSpacing(spacing) } : {})}>
<div ref={wrapperRef} className={wrapperClass}>
{children}
</div>
</Component>
) : (
<Component {...(spacing ? { className: getSpacing(spacing) } : {})}>
<div ref={wrapperRef} className={wrapperClass}>
{children}
</Component>
</div>
) : (
<>{children}</>
);
};
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import type { SpacingTypes } from "@versini/ui-types";

export type ThemeProviderProps = {
/**
* The children to render.
Expand All @@ -26,4 +24,4 @@ export type ThemeProviderProps = {
* @default false
*/
global?: boolean;
} & SpacingTypes.Props;
};
Original file line number Diff line number Diff line change
Expand Up @@ -23,31 +23,6 @@ describe("ThemeProvider props tests", () => {
const node = await screen.findByText("Hello World");
expectToHaveClasses(node, [THEMEPROVIDER_CLASSNAME, "contents", "toto"]);
});

it("should respect the spacing prop", async () => {
await act(async () => {
render(
<ThemeProvider className="toto" spacing={20}>
Hello World
</ThemeProvider>,
);
});
const node = await screen.findByText("Hello World");
expectToHaveClasses(node, [THEMEPROVIDER_CLASSNAME, "contents", "toto"]);
expect(node.parentElement).toHaveClass("m-20");
});

it("should respect the spacing prop even if global is true", async () => {
await act(async () => {
render(
<ThemeProvider className="toto" spacing={20} global>
Hello World
</ThemeProvider>,
);
});
const node = await screen.findByText("Hello World");
expect(node).toHaveClass("m-20");
});
});

describe("ThemeProvider injection tests", () => {
Expand Down
3 changes: 0 additions & 3 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 0586551

Please sign in to comment.