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

질문 작성 API를 연결한다. #35

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from
Open

질문 작성 API를 연결한다. #35

wants to merge 15 commits into from

Conversation

warmwhiten
Copy link
Member

@warmwhiten warmwhiten commented Feb 1, 2021

  • 질문 작성 API를 연결했습니다.

    • 로그인 했을 때만 질문 작성이 가능하도록 구현했습니다.
  • baseurl과 관련 설정을 변경했습니다.

추가로 필요한 작업

  • 🌟 Questioner가 prop(혹은 contextAPI 이용)을 받아 유저 정보를 반영할 수 있도록 변경
  • 🐛 질문 작성 후 useRouter를 이용해 새로고침을 해주는데, 질문 목록이 갱신되지 않음. 직접 새로고침을 한 번 더 눌러야 반영되는 문제 발생
  • 🐛 질문 목록에서 개별 질문 아이템을 클릭했을 때 새로운 페이지로의 연결이 정상동작 되지 않았다가 새로고침 후 몇 번 더 누르니 정상동작 함.

@warmwhiten warmwhiten linked an issue Feb 1, 2021 that may be closed by this pull request
2 tasks
@warmwhiten warmwhiten changed the title Feature/33 질문 작성 API를 연결한다. Feb 1, 2021
@warmwhiten warmwhiten self-assigned this Feb 1, 2021
Copy link
Member

@toneyparky toneyparky left a comment

Choose a reason for hiding this comment

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

거뜬하게 구현해주셨네요~
수고하셨습니다😊

}

setIsFolded(false);

Copy link
Member

Choose a reason for hiding this comment

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

불필요한 공백을 발견했슴니당😬

Copy link
Member Author

Choose a reason for hiding this comment

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

지금 제 eslint와 prettier 설정이 비정상적인 부분이 많은데..... 다음 프로젝트 들어갈 땐 확실히 설정하고 시작해야겠어요! 반영하겠습니다 😃

Copy link
Member

Choose a reason for hiding this comment

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

좋습니다!

@@ -1,10 +1,14 @@
import axios from "axios";
import {TOKEN, TOKEN_TYPE} from "./token";

const BASE_URL = 'http://localhost:8080';
const BASE_URL = 'http://13.124.134.56:8080';
Copy link
Member

Choose a reason for hiding this comment

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

👏👏👏👏👏

Copy link
Member

@kouz95 kouz95 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 👍
리뷰 확인해주세요~ 🙂

Comment on lines 79 to 80
const [isFolded, setIsFolded] = useState<boolean>(true);
const [title, setTitle] = useState<string>('');
Copy link
Member

Choose a reason for hiding this comment

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

initialState로 type을 추론할 수 있지 않을까요?
이런 경우엔 타입을 생략하는 걸 선호하는데, 어떻게 생각하시나요?

Suggested change
const [isFolded, setIsFolded] = useState<boolean>(true);
const [title, setTitle] = useState<string>('');
const [isFolded, setIsFolded] = useState(true);
const [title, setTitle] = useState('');

Copy link
Member Author

Choose a reason for hiding this comment

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

일단 추론 가능한 경우에 type 생략을 선호하신다는 것에 충분히 공감합니다. 'type을 정해주어야한다'는 강박에 매몰되는 걸 경계해야겠구나 하고 생각했어요.
그러면서 타입스크립트를 사용하는 이유에 대해 다시 한 번 생각해보았는데요, 예측 불가능한 실수를 미리 막는다는 점에서 이러한 경우에도 타입을 명시하는 게 더 좋지 않을까(제가 코드를 작성하며 어떤 실수를 할 지 모르니..) 하고 생각했어요. 또 레포를 몇 개 돌아다니면서 봤는데 명시해주는 경우가 더 많은 것 같더라고요. 그래서 그러한 경우에도 저는 타입을 명시해줄 것 같습니다!

Copy link
Member

Choose a reason for hiding this comment

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

제 pr에 남겨두셨던 코멘트와 비슷한 내용이군요.

어떻게하면 좋을지 생각해보죠!
저번에는 생략쪽에 기울었는데 다희님께서 다른 레포를 참고하시고 명시하는 경우가 더 많다니까 다시 고려해볼만 하네요.

Copy link
Member

Choose a reason for hiding this comment

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

정답은 없는 부분이니 다음에 이야기 나눠보죠 🙂

} else if (!mainContent) {
alert('질문 내용을 입력해주세요.');
return true;
} else if (!isLogin) {
Copy link
Member

Choose a reason for hiding this comment

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

로그인이 안되어있으니 로그인 페이지로 보내는것도 좋을것 같아요

Copy link
Member Author

Choose a reason for hiding this comment

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

오옹 좋은 제안입니다. 다음 커밋에 반영할게요!

Copy link
Member

Choose a reason for hiding this comment

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

좋네용
반영해서 푸쉬하면 될 듯 합니다😀

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.

질문 작성 API를 연결한다.
3 participants