[이나경] sprint4 #133
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,9 @@ | ||
| { | ||
| "liveServer.settings.port": 5501, | ||
| "github.copilot.enable": { | ||
| "*": false, | ||
| "plaintext": false, | ||
| "markdown": false, | ||
| "scminput": false | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| <!DOCTYPE html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="UTF-8" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
| <title>items</title> | ||
| </head> | ||
| <body> | ||
| items | ||
| </body> | ||
| </html> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| export const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; | ||
|
|
||
| /* 에러 메시지 */ | ||
| export function showError(input, error, message) { | ||
|
|
||
| input.classList.remove('success'); | ||
| input.classList.add('error'); | ||
| error.textContent = message; | ||
| error.classList.remove('hidden'); | ||
| } | ||
|
|
||
| /* 성공 메시지 */ | ||
| export function showSuccess(input, error) { | ||
| input.classList.add('success'); | ||
| input.classList.remove('error'); | ||
| error.classList.add('hidden'); | ||
| } | ||
|
|
||
| /* 이메일 유효성 검사 */ | ||
| export function validationEmail(input, error) { | ||
|
Collaborator
There was a problem hiding this comment. 로그인/회원가입에서 공통되는 유효성 검사를 formvalidation.js로 분리하고, 비밀번호 토글 기능도 toggle.js로 따로 나눴습니다! 그런데 validation 함수뿐 아니라 버튼 상태 업데이트 함수나 페이지 이동 함수까지 같이 넣은 게 괜찮은 설계인지 궁금합니다. 혹시 지금 구조보다 더 나은 방식이 있다면 어떻게 나눌 수 있을지도 조언해주시면 감사하겠습니다! |
||
| const value = input.value.trim(); | ||
|
|
||
| if (value === '') { | ||
| showError(input, error, '이메일을 입력해주세요'); | ||
| } else if (!emailRegex.test(value)) { | ||
| showError(input, error, '잘못된 이메일 형식입니다'); | ||
| } else { | ||
| showSuccess(input, error); | ||
| } | ||
| } | ||
|
|
||
| /* 비밀번호 유효성 검사 */ | ||
| export function validationPassword(input, error) { | ||
|
Collaborator
There was a problem hiding this comment. 또 구현하면서 이메일, 닉네임, 비밀번호 등 유효성 검사 함수 내부 로직이 거의 비슷해서, 조건과 메시지만 다르게 공통 함수로 추출하는 게 더 나은 구조일까 고민이 들었습니다. 각각의 역할이 명확한 게 유지보수나 SOLID 원칙에 더 부합할지, 아니면 중복을 줄이기 위해 하나로 합치는 게 더 실무적으로 좋은 선택일지 궁금합니다! 오히려 좀 더 나누는 것은 가능할 거 같아요. 지금의 경우 함수가 유효성 검사도 하고 에러 핸들링도 다루고 있습니다. 그런데 만약 이메일 찾기 같은 페이지가 추가되고, 이메일 찾기 페이지에서 이메일 입력 에러 발생시 로그인 페이지와 다른 UI를 보여줘야 한다면 이 함수는 수정이 필요하겠죠..! 🤣 하지만 늘 정답은 없습니다..! 지금 상황에서는 충분히 잘 나누셨다고 생각해요. 만약 이 프로젝트가 그냥 여기서 끝난다면? 혹은 시간이 너무 부족하다면? 어느정도 단순하고 빠르게 개발하고 나중에 필요할 때 리팩토링 하는 것도 방법이 되겠죠 :) |
||
| const value = input.value.trim(); | ||
|
|
||
| if (value === '') { | ||
| showError(input, error, '비밀번호를 입력해주세요'); | ||
| } else if (value.length < 8) { | ||
| showError(input, error, '비밀번호를 8자 이상 입력해주세요'); | ||
| } else { | ||
| showSuccess(input, error); | ||
| } | ||
| } | ||
|
|
||
| /* 모든 입력값이 유효한지 확인 */ | ||
| export function inputsValid(input) { | ||
| for (let i = 0; i < input.length; i++) { | ||
|
Collaborator
There was a problem hiding this comment. for of, forEach 등을 활용하시는 것이 실수를 줄이고, 가독성에 조금 더 도움이 됩니다 :) |
||
| if (!input[i].classList.contains('success')) { | ||
| return false; | ||
| } | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| /* 버튼 활성화 */ | ||
| export function updateButtonState(input, button) { | ||
| button.disabled = !inputsValid(input); | ||
| } | ||
|
|
||
| /* 버튼 누르면 페이지 이동 */ | ||
| export function goToPage(input, button, url) { | ||
|
Collaborator
There was a problem hiding this comment. 이 부분은 공통으로 쓰이는 로직은 아닐 거 같아요~ 페이지 이동이기보다는 로그인, 회원 가입 등의 요청을 하고 응답을 처리하므로 각각 구현하게 될 거 같습니다 :) |
||
| button.addEventListener('click', (e) => { | ||
| e.preventDefault(); | ||
| if (inputsValid(input)) { | ||
| window.location.href = url; | ||
| } | ||
| }); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| import { | ||
| validationEmail, | ||
| validationPassword, | ||
| updateButtonState, | ||
| goToPage, | ||
| } from './formvalidation.js'; | ||
| const emailInput = document.getElementById('email'); | ||
| const pwInput = document.getElementById('password'); | ||
| const emailError = emailInput.nextElementSibling; | ||
| const pwError = pwInput.nextElementSibling; | ||
| const loginButton = document.querySelector('.auth__button'); | ||
|
|
||
| const inputs = [emailInput, pwInput]; | ||
|
|
||
| /* 이메일 검증 */ | ||
| function validateEmail() { | ||
| validationEmail(emailInput, emailError); | ||
| updateButtonState(inputs, loginButton); | ||
| } | ||
|
|
||
| /* 비밀번호 검증 */ | ||
| function validatePassword() { | ||
| validationPassword(pwInput, pwError); | ||
| updateButtonState(inputs, loginButton); | ||
| } | ||
|
|
||
| /* 페이지 이동 */ | ||
| goToPage(inputs, loginButton, '../html/items.html'); | ||
|
|
||
| emailInput.addEventListener('blur', validateEmail); | ||
|
Collaborator
There was a problem hiding this comment. 현재 유효성 검사를 각각의 input마다 blur/input 이벤트에 addEventListener로 바인딩하고 있는데, 이 구조가 너무 반복적이고 지저분하다는 느낌이 들어서 더 깔끔하게 작성할 수 있는 방법이 있을지 궁금합니다! const inputValidations = {
email: validateEmail,
password: validatePassword,
};
const $form = document.querySelector("form");
$form.addEventListener("input", (e) => {
const validation = inputValidations[e.target.id];
validation();
updateButtonState(inputs, loginButton);
}); |
||
| emailInput.addEventListener('input', validateEmail); | ||
| pwInput.addEventListener('blur', validatePassword); | ||
| pwInput.addEventListener('input', validatePassword); | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
정규식을 쓰셨군요! 👍