Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(EmptyState): make titleText optional #770

Merged
merged 3 commits into from
Sep 26, 2024
Merged

feat(EmptyState): make titleText optional #770

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
933e273
feat(EmptyState): make titleText optional
adamviktora Sep 18, 2024
92e0d86
refactor (PR review)
adamviktora Sep 25, 2024
f8a4c1a
refactor no.2
adamviktora Sep 25, 2024
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
3 changes: 0 additions & 3 deletions .../v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

EmptyStateHeader and EmptyStateIcon are now rendered internally within EmptyState and should only be customized using props. Content passed to the `icon` prop on EmptyState will also be wrapped by EmptyStateIcon automatically.

Additionally, the `titleText` prop is now required on EmptyState.

#### Examples

In:
Expand All @@ -17,4 +15,3 @@ Out:
```jsx
%outputExample%
```

50 changes: 14 additions & 36 deletions ...rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ ruleTester.run("emptyStateHeader-move-into-emptyState", rule, {
{
code: `import { EmptyState } from '@patternfly/react-core'; <EmptyState titleText="foo" icon={CubesIcon} />`,
},
{
// without an EmptyStateHeader or Title text
code: `import { EmptyState } from "@patternfly/react-core"; <EmptyState>Foo bar</EmptyState>`,
},
],
invalid: [
{
Expand Down Expand Up @@ -106,14 +110,13 @@ ruleTester.run("emptyStateHeader-move-into-emptyState", rule, {
} from "@patternfly/react-core";

export const EmptyStateHeaderMoveIntoEmptyStateInput = () => (
<EmptyState>
<EmptyStateHeader />
</EmptyState>
<EmptyState >
</EmptyState>
);
`,
errors: [
{
message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props, and the titleText prop is now required on EmptyState. You must manually supply a titleText prop to EmptyState, then you can rerun this codemod.`,
message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props.`,
type: "JSXElement",
},
],
Expand Down Expand Up @@ -143,7 +146,7 @@ ruleTester.run("emptyStateHeader-move-into-emptyState", rule, {
`,
errors: [
{
message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props, and the titleText prop is now required on EmptyState.`,
message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props.`,
type: "JSXElement",
},
],
Expand Down Expand Up @@ -175,7 +178,7 @@ ruleTester.run("emptyStateHeader-move-into-emptyState", rule, {
`,
errors: [
{
message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props, and the titleText prop is now required on EmptyState.`,
message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props.`,
type: "JSXElement",
},
],
Expand Down Expand Up @@ -211,7 +214,7 @@ ruleTester.run("emptyStateHeader-move-into-emptyState", rule, {
`,
errors: [
{
message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props, and the titleText prop is now required on EmptyState.`,
message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props.`,
type: "JSXElement",
},
],
Expand Down Expand Up @@ -249,32 +252,7 @@ ruleTester.run("emptyStateHeader-move-into-emptyState", rule, {
`,
errors: [
{
message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props, and the titleText prop is now required on EmptyState.`,
type: "JSXElement",
},
],
},
{
// without an EmptyStateHeader or titleText
code: `import { EmptyState } from "@patternfly/react-core";

export const EmptyStateHeaderMoveIntoEmptyStateInput = () => (
<EmptyState>
Foo bar
</EmptyState>
);
`,
output: `import { EmptyState } from "@patternfly/react-core";

export const EmptyStateHeaderMoveIntoEmptyStateInput = () => (
<EmptyState>
Foo bar
</EmptyState>
);
`,
errors: [
{
message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props, and the titleText prop is now required on EmptyState. You must manually supply a titleText prop to EmptyState`,
message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props.`,
type: "JSXElement",
},
],
Expand Down Expand Up @@ -489,7 +467,7 @@ ruleTester.run("emptyStateHeader-move-into-emptyState", rule, {
`,
errors: [
{
message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props, and the titleText prop is now required on EmptyState.`,
message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props.`,
type: "JSXElement",
},
],
Expand Down Expand Up @@ -540,7 +518,7 @@ ruleTester.run("emptyStateHeader-move-into-emptyState", rule, {
type: "JSXElement",
},
{
message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props, and the titleText prop is now required on EmptyState. Additionally, the EmptyStateIcon component now wraps content passed to the icon prop automatically.`,
message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props. Additionally, the EmptyStateIcon component now wraps content passed to the icon prop automatically.`,
type: "JSXElement",
},
],
Expand Down Expand Up @@ -587,7 +565,7 @@ ruleTester.run("emptyStateHeader-move-into-emptyState", rule, {
type: "JSXElement",
},
{
message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props, and the titleText prop is now required on EmptyState. Additionally, the EmptyStateIcon component now wraps content passed to the icon prop automatically.`,
message: `EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props. Additionally, the EmptyStateIcon component now wraps content passed to the icon prop automatically.`,
type: "JSXElement",
},
],
Expand Down
65 changes: 18 additions & 47 deletions .../src/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeader-move-into-emptyState.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
getFromPackage,
getChildrenAsAttributeValueText,
getRemoveElementFixes,
childrenIsEmpty,
} from "../../helpers";
// https://github.com/patternfly/patternfly-react/pull/9947

Expand All @@ -26,29 +27,11 @@ const composeMessage = (
hasChildren?: boolean
) => {
let message =
"EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props";
const missingTitleTextMessage =
", and the titleText prop is now required on EmptyState.";

if (hasTitleText) {
message += ".";
} else {
message += missingTitleTextMessage;
}
"EmptyStateHeader has been moved inside of the EmptyState component and is now only customizable using props.";

if (hasTitleText && hasChildren) {
message +=
" Because the children for EmptyStateHeader are now inaccessible you must remove either the children or the titleText prop";
} else if (!hasTitleText && !hasChildren) {
message += " You must manually supply a titleText prop to EmptyState";
}

const hasHeader = [hasTitleText, hasIcon, hasChildren].some(
(arg) => typeof arg !== "undefined"
);

if (hasTitleText === hasChildren && hasHeader) {
message += ", then you can rerun this codemod.";
" Because the children for EmptyStateHeader are now inaccessible you must remove either the children or the titleText prop, then you can rerun this codemod.";
}

if (hasIcon) {
Expand Down Expand Up @@ -175,19 +158,6 @@ module.exports = {

const titleChild = getChildJSXElementByName(node, "Title");

if (
(!header || header.type !== "JSXElement") &&
(!titleChild || titleChild.type !== "JSXElement")
) {
// report without fixer if there is no header/title or the header/title is not a React element, because
// creating a titleText for the EmptyState in this case is difficult
context.report({
node,
message: composeMessage(),
});
return;
}

const newEmptyStateProps: string[] = [];
const removeElements: JSXElement[] = [];

Expand All @@ -204,6 +174,10 @@ module.exports = {
"EmptyStateIcon"
);

if (!header && !titleChild && !emptyStateIconChild) {
return;
}

wise-king-sullyman marked this conversation as resolved.
Show resolved Hide resolved
let iconProp: string = "";

if (emptyStateIconChild) {
Expand Down Expand Up @@ -231,19 +205,12 @@ module.exports = {

hasTitleText = !!titleTextAttribute;
hasIcon ||= !!headerIconAttribute;
hasChildren ||= header.children.length > 0;
hasChildren ||= !childrenIsEmpty(header.children);
wise-king-sullyman marked this conversation as resolved.
Show resolved Hide resolved

const message = composeMessage(hasTitleText, hasIcon, hasChildren);

if (!titleTextAttribute && !hasChildren) {
// report without fixer if there is a header, but it doesn't have titleText or children, because creating a
// titleText for the EmptyState in this case is difficult
context.report({ node, message });
return;
}

if (titleTextAttribute && hasChildren) {
// report without fixer if there is the header has a titleText and children, because creating an accessible
// report without fixer if the header has both titleText and children, because creating an accessible
// titleText for the EmptyState in this case is difficult
context.report({ node, message });
return;
Expand All @@ -267,12 +234,16 @@ module.exports = {
titleTextAttribute
);

const createTitleTextPropFromChildren = () =>
hasChildren
? `titleText=${getChildrenAsAttributeValueText(
context,
header.children
)}`
: "";

const titleText =
titleTextPropValue ||
`titleText=${getChildrenAsAttributeValueText(
context,
header.children
)}`;
titleTextPropValue || createTitleTextPropFromChildren();

const iconPropValue = getExpression(headerIconAttribute?.value);

Expand Down
17 changes: 17 additions & 0 deletions ...c/rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeaderMoveIntoEmptyStateInput.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,20 @@ export const EmptyStateWithoutHeaderMoveIntoEmptyStateInput = () => (
<EmptyStateBody>Body</EmptyStateBody>
</EmptyState>
);

export const EmptyStateHeaderWithoutTitleTextMoveIntoEmptyStateInput = () => (
<EmptyState>
<EmptyStateHeader
className="some-class"
icon={<EmptyStateIcon icon={CubesIcon} />}
/>
</EmptyState>
);

export const EmptyStateWithoutHeaderAndTitleTextMoveIntoEmptyStateInput =
() => (
<EmptyState>
<EmptyStateIcon icon={CubesIcon} />
<EmptyStateBody>Body</EmptyStateBody>
</EmptyState>
);
12 changes: 12 additions & 0 deletions .../rules/v6/emptyStateHeaderMoveIntoEmptyState/emptyStateHeaderMoveIntoEmptyStateOutput.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,15 @@ export const EmptyStateWithoutHeaderMoveIntoEmptyStateInput = () => (
<EmptyStateBody>Body</EmptyStateBody>
</EmptyState>
);

export const EmptyStateHeaderWithoutTitleTextMoveIntoEmptyStateInput = () => (
<EmptyState headerClassName="some-class" icon={CubesIcon} >
</EmptyState>
);

export const EmptyStateWithoutHeaderAndTitleTextMoveIntoEmptyStateInput =
() => (
<EmptyState icon={CubesIcon}>
<EmptyStateBody>Body</EmptyStateBody>
</EmptyState>
);
Loading