-
Notifications
You must be signed in to change notification settings - Fork 0
feature: added markdowns list #6
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
base: main
Are you sure you want to change the base?
Conversation
| @@ -1,30 +1,29 @@ | |||
| import React from 'react'; | |||
| import React, {useEffect, useRef} from 'react'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to import react?
|
|
||
| function App() { | ||
|
|
||
| import Markdown from "./components/MarkdownComp"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| import Markdown from "./components/MarkdownComp"; |
|
|
||
| const MarkdownComp = memo(function MarkdownComp({text, title}: {text: string, title?: string | ""}) { | ||
|
|
||
| const converter = new showdown.Converter(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't use this variable. Do we need it?
| const MarkdownComp = memo(function MarkdownComp({text, title}: {text: string, title?: string | ""}) { | ||
|
|
||
| const converter = new showdown.Converter(); | ||
| const cleanHTML = sanitize(converter.makeHtml(text)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't use this variable. Do we need it?
| "@types/showdown": "^2.0.2", | ||
| "antd": "^5.7.3", | ||
| "d3": "^7.8.5", | ||
| "dompurify": "^3.0.6", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the library is not used
| "react-scripts": "5.0.1", | ||
| "remark-gfm": "^4.0.0", | ||
| "sass": "^1.69.0", | ||
| "showdown": "^2.1.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the library is not used
| "@testing-library/react": "^13.4.0", | ||
| "@testing-library/user-event": "^13.5.0", | ||
| "@types/d3": "^7.4.0", | ||
| "@types/dompurify": "^3.0.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the library is not used
| // } | ||
|
|
||
| const AboutMe = () => { | ||
| const [isModalOpen, setModalOpen] = useState(false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we use a different name for set. Something like updateIsModalOpen?
| <div style={{display: "flex"}}> | ||
| <Modal | ||
| open={isModalOpen} | ||
| okButtonProps={{ style: { display: "none"} }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move all the styles in appropriate file or in an external variable?
| width={2000} | ||
| > | ||
| <MarkdownComp text={markdownTextList[selectedMarkdown - 1]} /> | ||
| <Pagination simple total={30} onChange={(page)=> setSelectedMarkdown(page)} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we externalize the variable? maybe not an hard-coded number but related to markdowns' number
No description provided.