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.

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

Feat: useContext in Pencil's todo list #16

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

zoeGuava
Copy link
Contributor

終於....有勞了...
真相欸

Copy link
Member

@ashramwen ashramwen left a comment

Choose a reason for hiding this comment

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

context 通常會習慣獨立一個檔案
在這個 case,可以把 TodoList 中的 functions 和 state 都放到 context 這檔案中

src/pages/Senpai/index.js Show resolved Hide resolved
src/pages/Senpai/index.js Show resolved Hide resolved

function Layout() {
const [currentPath, setCurrentPath] = useState('');
const pathArr = useLocation().pathname.split('/senpai/');
const path = pathArr[pathArr.length - 1];
Copy link
Member

Choose a reason for hiding this comment

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

return (
<div className="min-h-screen w-full bg-stone-800">
<h1 className="text-white text-2xl">Pencil's React workshop dashboard</h1>
<nav className="flex justify-around box-border bg-cyan-700">
<Link
Copy link
Member

Choose a reason for hiding this comment

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

Comment on lines +16 to +17
id={ADD_TODO}
name={ADD_TODO}
Copy link
Member

Choose a reason for hiding this comment

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

這兩行有用途嗎?

Comment on lines +75 to +76
const [inputValue, setInputValue] = useState(item.text);
const { updateTodo, toggleDataState, deleteItem } = useMethods();
Copy link
Member

Choose a reason for hiding this comment

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

這兩行怎麼不在 EditAreaButtonArea 呼叫就好?

function EditArea({ item, methods, inputValue }) {
return (
<div className={clsx('flex w-0 grow')}>
{!item.isEdit && (
Copy link
Member

Choose a reason for hiding this comment

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

這可以跟下面的合併

Comment on lines +46 to +48
{item.isDone ? (
<p className={clsx('w-20 rounded-md', 'bg-stone-800 text-lime-300')}>Done</p>
) : item.isEdit ? (
Copy link
Member

Choose a reason for hiding this comment

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

這好複雜
但其實 isDone 跟 isEdit 並沒有交疊關係不是嗎?

{isEdit ? <.../> : <.../>}

{isDone ? <.../> : <.../> }

import { useState } from 'react';
import Button from './Button';

function TodoForm({ item, updateTodo, toggleDataState, deleteItem }) {
Copy link
Member

Choose a reason for hiding this comment

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

這隻檔案沒用到吧

障眼法?

);
}

export const useMethods = () => useContext(MethodContext);
Copy link
Member

Choose a reason for hiding this comment

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

全部東西都放在同一個檔案裡
context
custom hook
component
很複雜

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants