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.

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

fix(Card): updated markup for actionable cards #738

Merged
merged 4 commits into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import {
JSXElement,
JSXText,
JSXExpressionContainer,
JSXSpreadChild,
JSXFragment,
} from "estree-jsx";

function getChildJSXElementCallback(
childNode:
| JSXElement
| JSXText
| JSXExpressionContainer
| JSXSpreadChild
| JSXFragment,
name: string
) {
return (
childNode.type === "JSXElement" &&
childNode.openingElement.name.type === "JSXIdentifier" &&
childNode.openingElement.name.name === name
);
}

/** Can be used to run logic if the specific child element exists, or to run logic on the
* specified element.
*/
export function getChildJSXElementByName(node: JSXElement, name: string) {
return node.children?.find((child) =>
getChildJSXElementCallback(child, name)
) as JSXElement | undefined;
}

/** Can be used to run logic if the specific child elements exist, or to run logic on the
* specified elements.
*/
export function getAllChildJSXElementsByName(node: JSXElement, name: string) {
return node.children?.filter((child) =>
getChildJSXElementCallback(child, name)
) as JSXElement[];
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
import { ImportSpecifier } from "estree-jsx";
import { getSpecifierFromImports } from "./getSpecifierFromImports";

/** Resolves the local name of an import */
export function getLocalComponentName(
namedImports: ImportSpecifier[],
importedName: string
) {
const componentImport = namedImports.find(
(name) => name.imported.name === importedName
);

const componentImport = getSpecifierFromImports(namedImports, importedName);
const isAlias =
componentImport?.imported.name !== componentImport?.local.name;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { Rule } from "eslint";
import { JSXAttribute } from "estree-jsx";
import { getVariableDeclaration } from "./JSXAttributes";

/** Used to find the node where a prop value is initially assigned, to then be passed
* as a fixer function's nodeOrToken argument. Useful for when a prop may have an inline value, e.g. `<Comp prop="value" />`, or
* is passed an identifier, e.g. `const val = "value"; <Comp prop={val} />`
*/
export function getNodeForAttributeFixer(
context: Rule.RuleContext,
attribute: JSXAttribute
) {
if (!attribute?.value) {
return;
}

switch (attribute.value.type) {
case "Literal":
return attribute.value;
case "JSXExpressionContainer":
return getJSXExpressionContainerValue(context, attribute);
}
}

function getJSXExpressionContainerValue(
context: Rule.RuleContext,
attribute: JSXAttribute
) {
if (!attribute.value || attribute.value.type !== "JSXExpressionContainer") {
return;
}

switch (attribute.value.expression.type) {
case "ObjectExpression":
case "MemberExpression":
return attribute.value.expression;
case "Identifier":
const scope = context.getSourceCode().getScope(attribute);
const variableDeclaration = getVariableDeclaration(
attribute.value.expression.name,
scope
);

return variableDeclaration && variableDeclaration.defs[0].node.init;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { Rule } from "eslint";
import { Property, Identifier } from "estree-jsx";
import { getVariableDeclaration } from "./JSXAttributes";

/** Can be used to run logic on the specified property of an ObjectExpression or
* only if the specified property exists.
*/
export function getObjectProperty(
context: Rule.RuleContext,
properties: Property[],
name: string
) {
if (!properties.length) {
return;
}

const matchedProperty = properties.find((property) => {
const isIdentifier = property.key.type === "Identifier";
const { computed } = property;

// E.g. const key = "key"; {[key]: value}
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 for these examples in comments

if (isIdentifier && computed) {
const scope = context.getSourceCode().getScope(property);
const propertyName = (property.key as Identifier).name;
const propertyVariable = getVariableDeclaration(propertyName, scope);
return propertyVariable?.defs[0].node.init.value === name;
}
// E.g. {key: value}
if (isIdentifier && !computed) {
return (property.key as Identifier).name === name;
}
// E.g. {"key": value} or {["key"]: value}
if (property.key.type === "Literal") {
return property.key.value === name;
}
});

return matchedProperty;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { ImportSpecifier } from "estree-jsx";

/** Can be used to extract a specific specifier from an array of imports, such as to only
* run logic if X and Y imports are present or to use the specifier properties in other logic. */
export function getSpecifierFromImports(
imports: ImportSpecifier[],
importedName: string
) {
const importSpecifier = imports.find(
(imp) => imp.imported.name === importedName
);

return importSpecifier;
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,29 @@ export * from "./contextReports";
export * from "./findAncestor";
export * from "./fixers";
export * from "./getAttributeName";
export * from "./getChildJSXElementByName";
export * from "./getCodeModDataTag";
export * from "./getComponentImportName";
export * from "./getDefaultDeclarationString";
export * from "./getEndRange";
export * from "./getFromPackage";
export * from "./getImportedName";
export * from "./getLocalComponentName";
export * from "./getNodeForAttributeFixer";
export * from "./getNodeName";
export * from "./getObjectProperty";
export * from "./getSpecifierFromImports";
export * from "./getText";
export * from "./hasCodemodDataTag";
export * from "./helpers";
export * from "./importAndExport";
export * from "./includesImport";
export * from "./interfaces";
export * from "./JSXAttributes";
export * from "./JSXElements";
export * from "./nodeIsComponentNamed";
export * from "./nodeMatches";
export * from "./pfPackageMatches";
export * from "./removeElement";
export * from "./removeEmptyLineAfter";
export * from "./removePropertiesFromObjectExpression";
export * from "./renameProps";
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { JSXElement } from "estree-jsx";

export function nodeIsComponentNamed(node: JSXElement, componentName: string) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@adamviktora seems like you and I built similar helpers (this from you, checkMatchingJSXOpeningElement in the nodeMatches file from me)? Wdyt about us combining them into a single rule + if so, any preference on naming?

Copy link
Contributor

Choose a reason for hiding this comment

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

We can keep yours checkMatchingJSXOpeningElement, it is more general and we already have it as part of the initial general rule. I can refactor the two rules that are using this nodeIsComponentNamed to use checkMatchingJSXOpeningElement and then delete this helper.

if (node.openingElement.name.type === "JSXIdentifier") {
return node.openingElement.name.name === componentName;
}

return false;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { Property } from "estree-jsx";

/** Can be used to take the returned array and join it into a replacement string of object
* key:value pairs.
*/
export function removePropertiesFromObjectExpression(
currentProperties: Property[],
propertiesToRemove: (Property | undefined)[]
) {
if (!currentProperties) {
return [];
}
if (!propertiesToRemove) {
return currentProperties;
}

const propertyNamesToRemove = propertiesToRemove.map((property) => {
if (property?.key.type === "Identifier") {
return property.key.name;
}

if (property?.key.type === "Literal") {
return property.key.value;
}

return "";
});

const propertiesToKeep = currentProperties.filter((property) => {
if (property.key.type === "Identifier") {
return !propertyNamesToRemove.includes(property.key.name);
}

if (property.key.type === "Literal") {
return !propertyNamesToRemove.includes(property.key.value);
}

return false;
});

return propertiesToKeep;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
### card-updated-clickable-markup [(#10859)](https://github.com/patternfly/patternfly-react/pull/10859)

The markup for clickable-only cards has been updated. Additionally, the `selectableActions.selectableActionId` and `selectableActions.name` props are no longer necessary to pass to CardHeader for clickable-only cards. Passing them in will not cause any errors, but running the fix for this rule will remove them.

#### Examples

In:

```jsx
%inputExample%
```

Out:

```jsx
%outputExample%
```
Loading
Loading