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

Roomsetting, Paymentlist #46

Merged
merged 6 commits into from
Jun 5, 2019
Merged

Roomsetting, Paymentlist #46

merged 6 commits into from
Jun 5, 2019

Conversation

y123ob
Copy link
Collaborator

@y123ob y123ob commented Jun 4, 2019

RoomSetting 과 PaymentList를 만들었습니다.
Layer부분이 연결되는 로직은 아직 구현되지 않았습니다.
action/sagas 부분에 보강이 필요합니다.

Copy link
Collaborator

@GBS-Skile GBS-Skile left a comment

Choose a reason for hiding this comment

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

몇가지 사소한 문법 오류가 있긴 한데 이거 고치신 다음에는 그냥 직접 머지해도 괜찮을듯! 수고 많았어요

frontend/src/containers/RoomSettingPage.js Outdated Show resolved Hide resolved
@@ -19,6 +20,10 @@ import { ROOM_CREATE_SUCCESS } from './actions';
...state,
roomname: action.roomname,
}
case ROOM_SETTING_REQUEST:
return{
initialState
Copy link
Collaborator

Choose a reason for hiding this comment

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

return { initialState } 대신 return { ...initialState} 구문을 사용해야합니다
initialState는 object이기 때문에 지금 표현과 같이 쓰면 중첩된 object가 만들어집니다

<Block>
<List>
{
paymentlist & paymentlist.map(
Copy link
Collaborator

Choose a reason for hiding this comment

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

&가 아니라 &&여야 short circuit evaluation이 작동합니다.
(저렇게되면 paymentlistundefined일때도 좌항의 .map 함수가 실행되어 ReferenceError가 발생할 것입니다.)

@@ -28,6 +28,8 @@ const App = () => {
<Route path="/user/create_room/" component={RoomCreatePage} />
<Route path="/user/:room/entrance/" component={EntrancePage} />
<Route path="/room/" component={RoomPage} />
<Route path="/user/:room/setting/" component={RoomSettingPage} />
<Route path="/user/:room/payment_list/" component={PaymentListPage} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

의미상 /user/ 말고 /room/접두사가 붙는게 더 합당해 보입니다

frontend/src/components/pages/PaymentListPage/index.js Outdated Show resolved Hide resolved
@y123ob
Copy link
Collaborator Author

y123ob commented Jun 4, 2019

위 수정사항 수정했습니다.
state to props 부분만 확인부탁드려요!
오늘 내로 conflict 수정하고 머지 해 보겠습니다.

@GBS-Skile GBS-Skile merged commit 471ad68 into master Jun 5, 2019
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