Skip to content

feature: add route setting and login page #13

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.

Sign up for GitHub

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

Jump to bottom
Open
zoeGuava wants to merge 11 commits into
base: dev
Choose a base branch
from
Open

feature: add route setting and login page #13

zoeGuava wants to merge 11 commits into from

Conversation

@zoeGuava
Copy link
Collaborator

@zoeGuava zoeGuava commented Jun 26, 2022

  • add route setting
  • add mock account information in JSON file

@zoeGuava zoeGuava requested a review from ezcomenezgo June 26, 2022 08:47
@zoeGuava zoeGuava added the enhancement New feature or request label Jun 26, 2022
@zoeGuava
Copy link
Collaborator Author

zoeGuava commented Jun 29, 2022 •
edited
Loading

  1. remove console.log()
  2. don't save sensitive data with localstorage, please use the cookie to do that
  3. the file here utils/Constant.js can replace by a constant folder, and put js file here
  4. your header tag is repeated...

zoeGuava
zoeGuava commented Jun 29, 2022
View reviewed changes
src/pages/Login/Login.jsx
<h1>登入</h1>
<form action="submit">
<label>
<form action="submit" onSubmit={handleLogin} className="flex flex-col">
Copy link
Collaborator Author

@zoeGuava zoeGuava Jun 29, 2022

Choose a reason for hiding this comment

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

the onSubmit will disable form action="submit", so here is unnecessary to use action here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ezcomenezgo ezcomenezgo Awaiting requested review from ezcomenezgo

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@zoeGuava