Add Collapse #27
Add Collapse #27
Conversation
|
@aftabrehan please review. |
apps/views/pages/Collapse.jsx
Outdated
| <> | ||
| <Collapse | ||
| variant="primary" | ||
| lablecontent="Some placeholder content for the collapse component. This panel is hidden by default but revealed when the user activates the relevant trigger. | ||
| " | ||
| lable="toggle" | ||
| /> | ||
| </> |
There was a problem hiding this comment.
Empty Fragments <></> are necessary when there are multiple tags to return, as their is only one tag/component to return.
So it's unnecessary to use Empty Fragments <></> in this case.
| <> | |
| <Collapse | |
| variant="primary" | |
| lablecontent="Some placeholder content for the collapse component. This panel is hidden by default but revealed when the user activates the relevant trigger. | |
| " | |
| lable="toggle" | |
| /> | |
| </> | |
| <Collapse | |
| variant="primary" | |
| lablecontent="Some placeholder content for the collapse component. This panel is hidden by default but revealed when the user activates the relevant trigger." | |
| lable="toggle" | |
| /> |
apps/views/pages/Collapse.jsx
Outdated
| @@ -0,0 +1,16 @@ | |||
| import { Collapse } from "kbs-core"; | |||
|
|
|||
| function Collap() { | |||
There was a problem hiding this comment.
Can't figure out, why this named collap
apps/views/pages/index.jsx
Outdated
| <br /> | ||
| <Button>Hello</Button> | ||
| <br /> | ||
| <Link href="/Collapse">Collapse</Link> |
There was a problem hiding this comment.
It's a best practice sort your component link alphabatically.
So in future it'll save a lot of time.
| <br /> | |
| <Button>Hello</Button> | |
| <br /> | |
| <Link href="/Collapse">Collapse</Link> | |
| <br /> | |
| <Link href="/Collapse">Collapse</Link> | |
| <br /> | |
| <Button>Hello</Button> |
apps/views/pages/Collapse.jsx
Outdated
| ); | ||
| } | ||
|
|
||
| export default Collap; |
There was a problem hiding this comment.
Use capital letter only for the compnent name. Not for the usage file name.
| </button> | ||
|
|
||
| <div className="collapse" id="collapseExample"> | ||
| <div className="card card-body">{props.labelcontent}</div> |
There was a problem hiding this comment.
Always use camelCase notation for this type of cases.
| <div className="card card-body">{props.labelcontent}</div> | |
| <div className="card card-body">{props.labelContent}</div> |
There was a problem hiding this comment.
It can be renamed as content 👍
| <div className="card card-body">{props.labelcontent}</div> | |
| <div className="card card-body">{props.content}</div> |
| <div className="collapse" id="collapseExample"> | ||
| <div className="card card-body">{props.labelcontent}</div> | ||
| </div> |
There was a problem hiding this comment.
Is that another div important?
Or We use just one div.
as:
| <div className="collapse" id="collapseExample"> | |
| <div className="card card-body">{props.labelcontent}</div> | |
| </div> | |
| <div className="card card-body collapse" id="collapseExample"> | |
| {props.labelcontent} | |
| </div> |
What you think about it?
Any suggestion?
packages/kbs-core/src/index.tsx
Outdated
| @@ -1,2 +1,4 @@ | |||
| import * as React from "react"; | |||
| import Collapse from "./components/Collapse"; | |||
There was a problem hiding this comment.
It's a good practice to have an empty line between imports & exports. 👍
| import Collapse from "./components/Collapse"; | |
| import Collapse from "./components/Collapse"; | |
Fixes #8