-
Notifications
You must be signed in to change notification settings - Fork 6
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
test: 스토리북에 구글 맵 적용 후 검색 창 스토리북을 개선한다 #926
base: develop
Are you sure you want to change the base?
Conversation
<React.Fragment> | ||
<QueryClientProvider client={queryClient}> | ||
<GlobalStyle /> | ||
<MemoryRouter initialEntries={['/']}> | ||
<Wrapper | ||
apiKey={ | ||
process.env.NODE_ENV === 'production' | ||
? process.env.GOOGLE_MAPS_API_KEY_PROD! | ||
: process.env.GOOGLE_MAPS_API_KEY_DEV! | ||
} | ||
libraries={['marker']} | ||
> | ||
<Story /> | ||
</Wrapper> | ||
</MemoryRouter> | ||
</QueryClientProvider> | ||
</React.Fragment> |
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.
<React.Fragment> | |
<QueryClientProvider client={queryClient}> | |
<GlobalStyle /> | |
<MemoryRouter initialEntries={['/']}> | |
<Wrapper | |
apiKey={ | |
process.env.NODE_ENV === 'production' | |
? process.env.GOOGLE_MAPS_API_KEY_PROD! | |
: process.env.GOOGLE_MAPS_API_KEY_DEV! | |
} | |
libraries={['marker']} | |
> | |
<Story /> | |
</Wrapper> | |
</MemoryRouter> | |
</QueryClientProvider> | |
</React.Fragment> | |
<QueryClientProvider client={queryClient}> | |
<GlobalStyle /> | |
<MemoryRouter initialEntries={['/']}> | |
<Wrapper | |
apiKey={ | |
process.env.NODE_ENV === 'production' | |
? process.env.GOOGLE_MAPS_API_KEY_PROD! | |
: process.env.GOOGLE_MAPS_API_KEY_DEV! | |
} | |
libraries={['marker']} | |
> | |
<Story /> | |
</Wrapper> | |
</MemoryRouter> | |
</QueryClientProvider> |
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.
<React.Fragment>
요거슨 저번에도 얘기하긴 했지만 React import 안하면 오류 메시지 나는 것 때문에 넣어놨습니다. React.Fragment 대신 다른 방법으로 오류 메시지 없애는 방법은 없을까여?🧐
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.
제가 풀 땡겨서 확인했을 땐 오류가 없는데 어떤 오류인가여?
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.
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.
지금 제 윈도우 환경에서는 안뜨는데 맥에서도 확인해볼게여
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.
왜 안나는지 원인을 찾았습니다. 제가 웹스톰을 써서 경고를 자동으로 처리해주는 것 같아요.
- 윈도우 맥 둘다 웹스톰에선 import, Fragment 둘 다 지워도 경고 안뜹니다
- vscode에서는
import React from 'react';
만 살려두는 경우에 경고가 안뜹니다
혹시 import도 제거하셨나여?
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.
넴 저장시 안 쓰는 import는 자동으로 없어지는 게 제 vscode 설정이라 React를 쓰고 있는 곳이 없으면 지워집니다. import React from 'react';
를 살려두려면 일부러 저장하지 않아야 하는데, 그렇게 하는 것보다는 Fragment 태그를 두는 게 편할 것 같아서 넣어놨어요
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.
무슨 말인지 알 것 같네요..
Storybook은 별도의 번들링 프로세스를 구성해서 모듈로 취급되는 관계로, React를 전역으로 가져오려면 import React from 'react';
가 반드시 있어야 하는데, vscode의 미사용 import 제거를 기능을 사용하려다 보니 preview.tsx 내의 import도 자동으로 제거되어서 억지로 Fragment를 쓰고 있는거군요..
그런데 제 vscode에서는 다른 import 문장들과는 다르게 해당 react import 문장은 자동으로 제거되지 않는데 내일 한번 확인이 필요할 것 같아요
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.
맞습니다
저만 지워지는 거 였나요😂 낼 확인해봅시다
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.
네넹
<link href="https://fonts.googleapis.com" rel="preconnect" /> | ||
<link crossorigin href="https://fonts.gstatic.com" rel="preconnect" /> | ||
<link | ||
href="https://fonts.googleapis.com/css2?family=Noto+Sans+KR:wght@400;500;700&display=swap" | ||
href="https://fonts.googleapis.com/css2?family=Noto+Sans+KR:wght@400;500;700&display=fallback" | ||
rel="preload" | ||
as="style" | ||
/> | ||
<link | ||
href="https://fonts.googleapis.com/css2?family=Noto+Sans+KR:wght@400;500;700&display=swap" | ||
rel="stylesheet" | ||
as="font" | ||
onload="this.onload=null; this.rel='stylesheet'" |
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.
좋네요
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.
제안 사항만 확인해주세요.
이상 없는 것 같아서 어프룹 날립니다.
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.
리뷰 하나 확인 부탁드릴게요~
return ( | ||
<React.Fragment> | ||
<QueryClientProvider client={queryClient}> | ||
<GlobalStyle /> | ||
<MemoryRouter initialEntries={['/']}> | ||
<Wrapper | ||
apiKey={ | ||
process.env.NODE_ENV === 'production' | ||
? process.env.GOOGLE_MAPS_API_KEY_PROD! | ||
: process.env.GOOGLE_MAPS_API_KEY_DEV! | ||
} | ||
libraries={['marker']} | ||
> | ||
<Story /> | ||
</Wrapper> | ||
</MemoryRouter> | ||
</QueryClientProvider> | ||
</React.Fragment> |
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.
그냥 정말 개인적인 생각인데 컴포넌트 단위 렌더링을 위해 스토리북을 사용하는데 구글 지도가 이 과정에 포함되는게 조금 어색하다는 생각이 듭니다. 컴포넌트 단위로 렌더링을 할 수 있게 컴포넌트에서 구글 지도 api에 대한 참조를 끊어내는것도 괜찮지 않을까 하는 생각입니다! 혹시 어떻게 생각하시나요...??
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.
요거슨 길어질 것 같으니 만나서 얘기합시다
📄 Summary
map.style.visibility = 'hidden'; 하지 않아도 됩니다.
하지만 구글맵이 있는 스토리에 갔다가 없는 스토리에 갔을 때 그대로 구글 맵이 보이는 현상이 있어 이를 개선 후 없앨 예정입니다.
(새로고침하면 다시 사라지긴 하지만, 불편할 것 같아서 일단 안 보이게 조치했습니다)
관련 커밋 : 06cb35c
🕰️ Actual Time of Completion
🙋🏻 More
🚀 Storybook
close #925