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

Part 2: fixed linter issues #342

Merged
merged 1 commit into from
Jun 26, 2024
Merged

Part 2: fixed linter issues #342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 3 additions & 2 deletions .husky/pre-commit
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/bin/sh
#!/usr/bin/env sh
. "$(dirname -- "$0")/_/husky.sh"

echo "Linting your changes..."
npx pretty-quick --staged
npm run lint
#! npm run lint

1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
"clean": "rimraf dist",
"chromatic": "chromatic --exit-zero-on-changes",
"patch": "npm version patch",
"prepare": "husky install",
"sb": "storybook dev -p 6006",
"storybook": "storybook dev -p 6006",
"build-storybook": "storybook build",
Expand Down
28 changes: 0 additions & 28 deletions src/components/ui/Accordion/Accordion.stories.js
View file
Open in desktop

This file was deleted.

66 changes: 35 additions & 31 deletions src/components/ui/Accordion/Accordion.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,38 +1,42 @@
import React, {useState} from 'react';
import AccordionRoot from './shards/AccordionRoot';
import AccordionItem from './shards/AccordionItem';
import AccordionHeader from './shards/AccordionHeader';
import AccordionTrigger from './shards/AccordionTrigger';
import AccordionContent from './shards/AccordionContent';
import React, { useState } from 'react'
import AccordionRoot from './shards/AccordionRoot'
import AccordionItem from './shards/AccordionItem'
import AccordionHeader from './shards/AccordionHeader'
import AccordionTrigger from './shards/AccordionTrigger'
import AccordionContent from './shards/AccordionContent'

export type AccordionProps = {
items: {content: any}[];
items: { content: any }[]
}

const Accordion = ({items} : AccordionProps) => {
const [activeIndex, setActiveIndex] = useState<number>(-1);
const handleClick = (index: number) => {
setActiveIndex(activeIndex === index ? -1 : index);
};
const Accordion = ({ items }: AccordionProps) => {
const [activeIndex, setActiveIndex] = useState<number>(-1)
const handleClick = (index: number) => {
setActiveIndex(activeIndex === index ? -1 : index)
}

return (
<AccordionRoot>
{items.map((item, index) => (
<AccordionItem value={index}>
<AccordionHeader>
<AccordionTrigger handleClick={handleClick} index={index} activeIndex={activeIndex} >
Item {index+1}
</AccordionTrigger>
</AccordionHeader>
<AccordionContent index={index} activeIndex={activeIndex}>
{item.content}
</AccordionContent>
</AccordionItem>
))}
</AccordionRoot>
);
};
return (
<AccordionRoot>
{items.map((item, index) => (
<AccordionItem value={index}>
<AccordionHeader>
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a key property to the list items.

The React documentation and best practices recommend using a unique key for list items to help React track each component's identity during updates. This is missing in the map function for AccordionItem.

-        <AccordionItem value={index}>
+        <AccordionItem key={index} value={index}>
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<AccordionItem value={index}>
<AccordionItem key={index} value={index}>
Tools
Biome

[error] 21-21: Missing key property for this element in iterable. (lint/correctness/useJsxKeyInIterable)

The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.

<AccordionTrigger
handleClick={handleClick}
index={index}
activeIndex={activeIndex}
>
Item {index + 1}
</AccordionTrigger>
</AccordionHeader>
<AccordionContent index={index} activeIndex={activeIndex}>
{item.content}
</AccordionContent>
</AccordionItem>
))}
</AccordionRoot>
)
}

Accordion.Root = AccordionRoot;
Accordion.Root = AccordionRoot

export default Accordion;
export default Accordion
52 changes: 28 additions & 24 deletions src/components/ui/Accordion/shards/AccordionContent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,32 @@
import React from 'react';
// @ts-ignore
import {customClassSwitcher} from '~/core';
import React from 'react'
import { customClassSwitcher } from '~/core'

type AccordionContentProps = {
children: React.ReactNode;
index: number,
activeIndex: number,
customRootClass? :string
};
children: React.ReactNode
index: number
activeIndex: number
customRootClass?: string
}

const AccordionContent: React.FC<AccordionContentProps> = ({children, index, activeIndex, customRootClass}: AccordionContentProps) => {
const rootClass = customClassSwitcher(customRootClass, 'Accordion');
return (
<span className={`${rootClass}-content`}>
<div
id={`content-${index}`}
role="region"
aria-labelledby={`section-${index}`}
hidden={activeIndex !== index}
>
{children}
</div>
</span>
);
};
const AccordionContent: React.FC<AccordionContentProps> = ({
children,
index,
activeIndex,
customRootClass
}: AccordionContentProps) => {
const rootClass = customClassSwitcher(customRootClass, 'Accordion')
return (
<span className={`${rootClass}-content`}>
<div
id={`content-${index}`}
role="region"
aria-labelledby={`section-${index}`}
hidden={activeIndex !== index}
>
{children}
</div>
</span>
)
}

export default AccordionContent;
export default AccordionContent
26 changes: 12 additions & 14 deletions src/components/ui/Accordion/shards/AccordionHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,17 @@
import React from 'react';
// @ts-ignore
import {customClassSwitcher} from '~/core';
import React from 'react'
import { customClassSwitcher } from '~/core'

export type AccordionHeaderProps = {
children: React.ReactNode;
customHeaderClass?: string;
children: React.ReactNode
customHeaderClass?: string
}

const AccordionHeader: React.FC<AccordionHeaderProps> = ({children, customHeaderClass=''}) => {
const rootClass = customClassSwitcher(customHeaderClass, 'Accordion');
return (
<div className={`${rootClass}-header`}>
{children}
</div>
);
};
const AccordionHeader: React.FC<AccordionHeaderProps> = ({
children,
customHeaderClass = ''
}) => {
const rootClass = customClassSwitcher(customHeaderClass, 'Accordion')
return <div className={`${rootClass}-header`}>{children}</div>
}

export default AccordionHeader;
export default AccordionHeader
28 changes: 13 additions & 15 deletions src/components/ui/Accordion/shards/AccordionItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,18 @@
import React from 'react';
// @ts-ignore
import {customClassSwitcher} from '~/core';
import React from 'react'
import { customClassSwitcher } from '~/core'

export type AccordionItemProps = {
children: React.ReactNode;
customItemClass?: string;
value?: number;
children: React.ReactNode
customItemClass?: string
value?: number
}

const AccordionItem: React.FC<AccordionItemProps> = ({children, value, customItemClass=''}) => {
const rootClass = customClassSwitcher(customItemClass, 'Accordion');
return (
<div className={`${rootClass}-item`}>
{children}
</div>
);
};
const AccordionItem: React.FC<AccordionItemProps> = ({
children,
customItemClass = ''
}) => {
const rootClass = customClassSwitcher(customItemClass, 'Accordion')
return <div className={`${rootClass}-item`}>{children}</div>
}

export default AccordionItem;
export default AccordionItem
23 changes: 9 additions & 14 deletions src/components/ui/Accordion/shards/AccordionRoot.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,14 @@
import React from 'react';
// @ts-ignore
import {customClassSwitcher} from '~/core';
import React from 'react'
import { customClassSwitcher } from '~/core'

export type AccordionRootProps = {
children: React.ReactNode;
customRootClass?: string;
children: React.ReactNode
customRootClass?: string
}

const AccordionRoot= ({children, customRootClass}: AccordionRootProps) => {
const rootClass = customClassSwitcher(customRootClass, 'Accordion');
return (
<span className={`${rootClass}-root`}>
{children}
</span>
);
};
const AccordionRoot = ({ children, customRootClass }: AccordionRootProps) => {
const rootClass = customClassSwitcher(customRootClass, 'Accordion')
return <span className={`${rootClass}-root`}>{children}</span>
}

export default AccordionRoot;
export default AccordionRoot
53 changes: 28 additions & 25 deletions src/components/ui/Accordion/shards/AccordionTrigger.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,30 +1,33 @@
import React from 'react';
// @ts-ignore
import {customClassSwitcher} from '~/core';
import React from 'react'
import { customClassSwitcher } from '~/core'

type AccordionTriggerProps = {
children: React.ReactNode;
customRootClass?: string,
index: number,
activeIndex: number,
children: React.ReactNode
customRootClass?: string
index: number
activeIndex: number
handleClick: (index: number) => void
};
}

const AccordionTrigger: React.FC<AccordionTriggerProps> = ({children, handleClick, index, activeIndex, customRootClass}) => {
const rootClass = customClassSwitcher(customRootClass, 'Accordion');
return (
<span className={`${rootClass}-trigger`}>
const AccordionTrigger: React.FC<AccordionTriggerProps> = ({
children,
handleClick,
index,
activeIndex,
customRootClass
}) => {
const rootClass = customClassSwitcher(customRootClass, 'Accordion')
return (
<span className={`${rootClass}-trigger`}>
<button
onClick={() => handleClick(index)}
aria-expanded={activeIndex === index}
aria-controls={`content-${index}`}
>
{children}
Comment on lines +22 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

Add explicit button type to prevent form submission issues.

The button within the accordion trigger should explicitly specify its type to avoid unintentional form submissions when used within a form.

- <button
+ <button type="button"
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<button
onClick={() => handleClick(index)}
aria-expanded={activeIndex === index}
aria-controls={`content-${index}`}
>
<button
type="button"
onClick={() => handleClick(index)}
aria-expanded={activeIndex === index}
aria-controls={`content-${index}`}
>
Tools
Biome

[error] 22-26: Provide an explicit type prop for the button element. (lint/a11y/useButtonType)

The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset

</button>
</span>
)
}

<button
onClick={() => handleClick(index)}
aria-expanded={activeIndex === index}
aria-controls={`content-${index}`}
>
{children}
</button>

</span>
);
};

export default AccordionTrigger;
export default AccordionTrigger
Loading
Loading