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

스케줄 장소 포함 기능 #299

Merged
merged 1 commit into from
Mar 5, 2024
Merged

스케줄 장소 포함 기능 #299

merged 1 commit into from
Mar 5, 2024

Conversation

kikiyeom
Copy link
Member

@kikiyeom kikiyeom commented Mar 5, 2024

변경사항

  • 스케줄 생성, 수정 시 장소가 포함된다.

작업 유형

  • 신규 기능 추가

체크리스트

  • Merge 할 브랜치가 올바른가?
  • 코딩컨벤션을 준수하였는가?
  • 해당 PR과 관련없는 변경사항이 없는가? (만약 있다면 제목이나 변경사항에 기술하여 주세요.)
  • 실행시 console 창에 에러나 경고가 없는것을 확인하였는가? (개발에 필요하여 고의적으로 남겨둔것 제외)

Copy link

netlify bot commented Mar 5, 2024

Deploy Preview for affectionate-jones-aa31c6 ready!

Name Link
🔨 Latest commit b788281
🔍 Latest deploy log https://app.netlify.com/sites/affectionate-jones-aa31c6/deploys/65e72a0640a1770008971aaa
😎 Deploy Preview https://deploy-preview-299--affectionate-jones-aa31c6.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

longitude: roadAddress.x,
placeName: roadAddress.building_name ?? roadAddress.address_name,
});
setValue('placeName', roadAddress.building_name);
Copy link
Collaborator

Choose a reason for hiding this comment

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

앱에 building_name과 지도 핀만 노출하는 것이 기획이죠?
building_name이 ""인 경우는 어드민 장소 인풋에 입력되는 값이 없어서, 성공 여부를 알려줄 디자인이 있으면 좋을 것 같긴 합니다만.. 급하다면 패스해도 좋습니다

{
    "address_name": "경기 성남시 분당구 서판교로 32",
    "building_name": "",
    "main_building_no": "32",
    "region_1depth_name": "경기",
    "region_2depth_name": "성남시 분당구",
    "region_3depth_name": "판교동",
    "road_name": "서판교로",
    "sub_building_no": "",
    "underground_yn": "N",
    "x": "127.097880906475",
    "y": "37.389777093851",
    "zone_no": "13479"
}

Copy link
Member Author

Choose a reason for hiding this comment

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

아 이부분 놓쳤네요!
추가할게요!

@@ -40,6 +40,7 @@
/>
<meta property="og:image" content="/assets/og-image.png" />
<link rel="icon" href="/assets/favicon.svg" />
<script src="//t1.daumcdn.net/mapjsapi/bundle/postcode/prod/postcode.v2.js"></script>
Copy link
Collaborator

Choose a reason for hiding this comment

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

https://stackoverflow.com/a/60964007

해당 페이지에서 useScript와 같은 훅을 통해 동적 스크립트 및 async로 불러오는 게 어떨까요?
스크립트 사이즈가 얼마 안 되긴 하지만 스케줄 생성 페이지에서 한정적으로 사용되는 것으로 보여서요!

@kikiyeom
Copy link
Member Author

kikiyeom commented Mar 5, 2024

집 노트북 세팅이 온전치 못해서 리뷰해준 부분은 우선 현재 상태로 dev 배포하고 따로 추가 pr 만들게요!

@kikiyeom kikiyeom merged commit 1fd470e into develop Mar 5, 2024
6 checks passed
@kikiyeom kikiyeom deleted the feature/add-location branch March 5, 2024 14:21
@kikiyeom kikiyeom mentioned this pull request Mar 8, 2024
4 tasks
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