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

Feat/summary component #90

Merged
merged 4 commits into from
Jul 11, 2024
Merged

Feat/summary component #90

merged 4 commits into from
Jul 11, 2024

Conversation

mari1912
Copy link
Collaborator

Description

  • add summary component
  • add it to storybook
  • do some minor refactors in other components to make them more reusable

Related Issues

How to test it

Run the storybook

Visual reference

image

@mari1912 mari1912 requested a review from lfjnascimento July 11, 2024 14:40
@mari1912 mari1912 self-assigned this Jul 11, 2024
@@ -52,7 +54,12 @@ const TreeTable = ({ treeTableRows }: ITreeTable): JSX.Element => {
}, [treeTableRows]);

return (
<BaseTable headers={treeTableColumnsLabelId} body={<>{treeTableBody}</>} />
<BaseTable
headers={treeTableColumnsLabelId.map(columnLabelId => (
Copy link
Contributor

Choose a reason for hiding this comment

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

useMemo here please

</TableCell>
<TableCell>
<div className="flex flex-col gap-1">
{compilers.map(compiler => (
Copy link
Contributor

Choose a reason for hiding this comment

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

useMemo

- do this change to allow the table to receive a more general element, instead of a string containing an id to be used in FormattedMessage
@mari1912 mari1912 force-pushed the feat/summary-component branch from 4660c1c to fa5f548 Compare July 11, 2024 17:54
@@ -13,8 +13,10 @@ const containerClassName =
const BaseCard = ({ title, content, className }: IBaseCard): JSX.Element => {
return (
<div className={classNames(className, containerClassName)}>
Copy link
Contributor

Choose a reason for hiding this comment

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

It is the other way around classNames(containerClassName, className) that way the className prop can override the css properties

Comment on lines 30 to 38
const summaryBodyRows = useMemo(() => {
return summaryBody.map(row => (
<SummaryItem
key={row.arch.text}
arch={row.arch}
compilers={row.compilers}
/>
));
}, [summaryBody]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I feel like removing the unnecessary return will make everything cleaner, but it's up to you.

Suggested change
const summaryBodyRows = useMemo(() => {
return summaryBody.map(row => (
<SummaryItem
key={row.arch.text}
arch={row.arch}
compilers={row.compilers}
/>
));
}, [summaryBody]);
const summaryBodyRows = useMemo(
() =>
summaryBody.map(row => (
<SummaryItem
key={row.arch.text}
arch={row.arch}
compilers={row.compilers}
/>
)),
[summaryBody],
);

@mari1912 mari1912 force-pushed the feat/summary-component branch from fa5f548 to 9f3a2b4 Compare July 11, 2024 19:15
@mari1912 mari1912 merged commit 4386481 into main Jul 11, 2024
4 checks passed
@mari1912 mari1912 deleted the feat/summary-component branch July 11, 2024 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Summary component
2 participants