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

스케줄 상세주소 추가 작업 #300

Merged
merged 4 commits into from
Mar 8, 2024
Merged

Conversation

kikiyeom
Copy link
Member

@kikiyeom kikiyeom commented Mar 7, 2024

변경사항

작업 유형

  • 신규 기능 추가
  • 리팩토링

체크리스트

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

Copy link

netlify bot commented Mar 7, 2024

Deploy Preview for affectionate-jones-aa31c6 ready!

Name Link
🔨 Latest commit e761ec6
🔍 Latest deploy log https://app.netlify.com/sites/affectionate-jones-aa31c6/deploys/65eb0248c0cc7d0008b155e1
😎 Deploy Preview https://deploy-preview-300--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.

Copy link
Member

@HaJunRyu HaJunRyu left a comment

Choose a reason for hiding this comment

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

👍

method: 'GET',
headers: {
'Content-Type': 'application/json',
Authorization: 'KakaoAK cc4af66dc10aa1a20830f3cc62c40a87',
Copy link
Member

Choose a reason for hiding this comment

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

우리 public 레포인데 이건 이렇게 노출해도 되는건가?.?

Copy link
Member Author

Choose a reason for hiding this comment

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

띠용..😓 처리할게용

@@ -0,0 +1,15 @@
import { useEffect } from 'react';

const useScript = (url: string) => {
Copy link
Member

Choose a reason for hiding this comment

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

해당 hook에서 isScriptLoaded 같은 상태를 관리하고 hook 외부로 헤딩 싱태를 return 해주면 hook의 사용도가 더 올라갈거 같다는 생각!

Comment on lines +70 to +71
const baseAddress = address.replace(/\s*\(.*?\)\s*/g, '');
const [, detailAddress] = address.match(/\(([^)]+)\)/) ?? [];
Copy link
Member

Choose a reason for hiding this comment

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

이건 내 개인취향인데 정규식은 최대한 하는일에 맞게 이름을 지어주면 코드 파악이나 유지보수 할때 좋은거 같아! 사실 정규식이 한눈에 딱 보고 뭐하는지 알기가 힘드니ㅎㅎ

@kikiyeom kikiyeom merged commit a311aeb into develop Mar 8, 2024
5 checks passed
@kikiyeom kikiyeom deleted the refactor/add-location branch March 8, 2024 12:23
@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