mirageruler / df-frontend-2023

learning FE stuffs from dwarves
https://df-frontend-2023-rust.vercel.app
0 stars 1 forks source link

Submission for assignment 3 #3

Open mirageruler opened 1 year ago

mirageruler commented 1 year ago
tienan92it commented 1 year ago

Hello @mirageruler , well done!. Below is some feedback for your assignment.

Requirements

Final result: ✅ passed 80% of requirements

Feedback

  1. Enable noImplicitAny to reveal the implicit any types. https://github.com/mirageruler/df-frontend-2023/blob/af8c8c8c8d5c87b87b504f0733673c393938fa41/assignment-3/tsconfig.json#L15 https://github.com/mirageruler/df-frontend-2023/blob/af8c8c8c8d5c87b87b504f0733673c393938fa41/assignment-3/src/components/Modal.tsx#L3 https://github.com/mirageruler/df-frontend-2023/blob/af8c8c8c8d5c87b87b504f0733673c393938fa41/assignment-3/src/App.tsx#L45 https://github.com/mirageruler/df-frontend-2023/blob/af8c8c8c8d5c87b87b504f0733673c393938fa41/assignment-3/src/App.tsx#L58 ...

  2. Avoid disabling react-hooks/exhaustive-deps rule. In your case, just unify model state by using prop or local state only, don't need both open prop and openState. https://github.com/mirageruler/df-frontend-2023/blob/af8c8c8c8d5c87b87b504f0733673c393938fa41/assignment-3/src/components/Modal.tsx#L19 Suggestion:

    
    import React from "react";

type ModalProps = { title?: string; children?: React.ReactNode; open?: boolean; onClose: () => void; }

const Modal = (props: ModalProps) => { const { title = "", children, open = false, onClose } = props;

return open && (
    <div className="Modal-Background">
        <div className="Modal-Root">
            <div className="Modal-Header">
                <h1>{title}</h1>
                <button className="close" onClick={onClose}>&times;</button>
            </div>
            <div className="Modal-Body">{children}</div>
        </div>
    </div>
);

};

export default Modal;



3. Use `includes` instead of `startsWith` for better searching results.
https://github.com/mirageruler/df-frontend-2023/blob/af8c8c8c8d5c87b87b504f0733673c393938fa41/assignment-3/src/App.tsx#L34