PopUp Modal as separate component #129
Conversation
|
This pull request is automatically deployed with Now. |
ChaitanyaGadodia
changed the title
Codecov Report
@@ Coverage Diff @@
## master #129 +/- ##
==========================================
+ Coverage 74.13% 74.39% +0.25%
==========================================
Files 69 69
Lines 812 824 +12
Branches 157 161 +4
==========================================
+ Hits 602 613 +11
Misses 195 195
- Partials 15 16 +1
Continue to review full report at Codecov.
|
|
@ChaitanyaGadodia link to the design system. |
src/components/ConfirmationModal.tsx
Outdated
| No | ||
| </Button> | ||
| <Button size="large" type="primary" onClick={onApprove}> | ||
| Yes |
There was a problem hiding this comment.
- Create a
PopUpcomponent rather thanConfirmationModal. - Yes, No etc should be dynamic text taken as prop.
- you might not always have both buttons. Sometimes there can be simple text with "Ok, Got it" button.
- Get this added to design system.
src/components/PopUp.tsx
Outdated
| import * as styles from "./styles/PopUp.styles"; | ||
| import { PopUpProps } from "./typings/PopUp"; | ||
|
|
||
| export default class PopUp extends React.Component<PopUpProps> { |
There was a problem hiding this comment.
convert this into a functional component
src/components/PopUp.tsx
Outdated
| <Modal visible={visible}> | ||
| <div className={styles.modalContainer}> | ||
| <div | ||
| className={cx( |
There was a problem hiding this comment.
cx should only be used in case any conditional style is present
| minWidth: "360px", | ||
| alignSelf: "center", | ||
| border: `1px solid ${colors.gray.light}`, | ||
| borderRadius: "4px", |
|
|
||
| export const buttonsContainer = css({ | ||
| display: "flex", | ||
| justifyContent: "space-between", |
stories/modal.story.tsx
Outdated
| @@ -17,3 +17,12 @@ storiesOf("Modal", module).add("simple", () => ( | |||
| <div className={style} /> | |||
| </Modal> | |||
| )); | |||
|
|
|||
| storiesOf("Modal", module).add("PopUp", () => ( | |||
| background: "white", | ||
| minWidth: "360px", | ||
| alignSelf: "center", | ||
| border: `1px solid ${colors.gray.light}`, |
There was a problem hiding this comment.
don't see border in design.
stories/modal.story.tsx
Outdated
| visible={true} | ||
| headingText="Are you sure?" | ||
| onApprove={() => {}} | ||
| onClose={() => {}} |
src/components/PopUp.tsx
Outdated
| <div className={styles.buttonsContainer}> | ||
| {onClose && ( | ||
| <Button size="large" type="secondary" onClick={onClose}> | ||
| {closeButtonText || "No"} |
There was a problem hiding this comment.
you can assign defaultProps above itself
src/components/PopUp.tsx
Outdated
| import Modal from "./Modal"; | ||
| import Button from "./Button"; | ||
| import { mixins } from "../theme"; | ||
| import * as styles from "./styles/PopUp.styles"; |
There was a problem hiding this comment.
This is not actually styles but classNames. Returns a string. name might be confusing.
| <PopUp | ||
| visible={true} | ||
| headingText="Are you sure?" | ||
| onApprove={action("click")} |
There was a problem hiding this comment.
| onApprove={action("click")} | |
| onApprove={action("onApprove")} |
| visible={true} | ||
| headingText="Are you sure?" | ||
| onApprove={action("click")} | ||
| onClose={action("click")} |
There was a problem hiding this comment.
| onClose={action("click")} | |
| onClose={action("onClose")} |
ritz078
changed the title
https://gallery.io/projects/MCHbtQVoQ2HCZT4mqWZoe47O/files/MCHtA7U1iMGr68ZHkz_HyQvJNVoLINrU1MY