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

이벤트 코드 삭제 및 UI 버그 수정 #817

Merged
merged 7 commits into from
Oct 25, 2023
Merged

이벤트 코드 삭제 및 UI 버그 수정 #817

merged 7 commits into from
Oct 25, 2023

Conversation

chsua
Copy link
Collaborator

@chsua chsua commented Oct 25, 2023

🔥 연관 이슈

close: #816

📝 작업 요약

  • 이벤트용 마스코트 이벤트 삭제
  • IOS 사파리에서 게시글 정보가 중앙 정렬되는 오류 해결
  • IOS 사파리에서 버튼 글씨색이 파랑색으로 변하는 문제 해결(확실하지 않음)
  • 특정 IOS 사파리에서 켜지지 않는 문제 해결

⏰ 소요 시간

1일

  • 정규식 수정 오래 걸림
  • 로컬로 로그인 리다이렉트 해놓지 않아 테스트 못함

🔎 작업 상세 설명

불특정 사파리에서 Invalid regular expression: Invalid group specifier name 오류 발견

  • WebKit 기반의 브라우저에서 정규표현식의 lookbehind를 지원하지 않음 (2023.03월에 업데이트한 경우 지원됨)
  • 핸드폰에서 아예 api 통신이 되지 않아 머지되고 확인이 필요

🌟 공유사항

@chsua chsua self-assigned this Oct 25, 2023
@github-actions
Copy link

github-actions bot commented Oct 25, 2023

⚡️ Lighthouse report!

Category Score
🟠 Performance 72
🟠 Accessibilty 89
🟢 SEO 100
🟠 PWA 89
Category Score
🟢 First Contentful Paint 0.6 s
🟠 Largest Contentful Paint 3.0 s
🔴 Total Blocking Time 990 ms
🟢 Cumulative Layout Shift 0.046
🟢 Speed Index 3.0 s

Copy link
Member

@inyeong-kang inyeong-kang left a comment

Choose a reason for hiding this comment

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

덕분에 데모데이에 이벤트가 성공적이었네요🚀💯🔥
궁금한 점 코멘트로 남겼습니다!

// linkRegex: https:// | http:// | www. 뒤에 문자+숫자+특수기호가 이어져있는 정규표현식
const linkRegex = /(?:https?:\/\/|w{3}\.)+[a-z0-9-+&@#/%?=~_|!:,.;]*[a-z0-9-+&@#/%=~_|]/g;
// [[ ]]를 표현하는 정규표현식
const customLinkPattern = /\[\[([^[\]]+)\]\]/g;
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
Collaborator

Choose a reason for hiding this comment

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

주석 설명이 너무 좋아요 222

@@ -6,7 +6,8 @@ test.each([
'반갑다 https://github.com/woowacourse-teams/2023-votogether/issues/703 임',
'반갑다 [[https://github.com/woowacourse-teams/2023-votogether/issues/703]] 임',
],
['안녕 wwwww.naver.com', '안녕 wwwww.naver.com'],
['안녕 wwwww.naver.com', '안녕 ww[[www.naver.com]]'],
['하하 [www.naver.com]', '하하 [[[www.naver.com]]]'],
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
Collaborator Author

Choose a reason for hiding this comment

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

앞에 [[가 아니라면 자르고 하는게 맞다고 생각했어요!
예로, wowwww.---이런식으로 진짜 쓰진 않겠지만 쓸 수도 있을 수 있지 않을까요?

Copy link
Collaborator

@Gilpop8663 Gilpop8663 left a comment

Choose a reason for hiding this comment

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

이벤트 코드 삭제부터 코드에 신경쓰신 부분이 넘 좋았어요 👍👍👍

@@ -80,7 +81,7 @@ button{
}

a{
color: inherit;
color: black;
Copy link
Collaborator

Choose a reason for hiding this comment

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

파랑색 글씨가 사라지겠네요 👍👍👍

// linkRegex: https:// | http:// | www. 뒤에 문자+숫자+특수기호가 이어져있는 정규표현식
const linkRegex = /(?:https?:\/\/|w{3}\.)+[a-z0-9-+&@#/%?=~_|!:,.;]*[a-z0-9-+&@#/%=~_|]/g;
// [[ ]]를 표현하는 정규표현식
const customLinkPattern = /\[\[([^[\]]+)\]\]/g;
Copy link
Collaborator

Choose a reason for hiding this comment

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

주석 설명이 너무 좋아요 222

Comment on lines 8 to 19
const parts = text.split(customLinkPattern);

return parts
.map(part => {
//linkRegex를 포함하지 않는다면 그대로 return
if (!linkRegex.test(part)) return part;

return wwwConvertedText;
return part.replace(linkRegex, url => {
return `[[${url}]]`;
});
})
.join('');
Copy link
Collaborator

Choose a reason for hiding this comment

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

기존에 있는 [[ ]] 를 없애고 https / http / www 로 시작하는 지 판별하는 로직이군요!

코드에서 제안하고 싶은 부분은 아래와 같은데요 return이 너무 많이 있어서 가독성이 조금 떨어지는 것 같아요 이 부분은 return을 생략하는 것은 어떠세요? 😀

  return part.replace(linkRegex, url => `[[${url}]]`;
      });

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

코드를 수정하다가 놓쳐버렸네요! 수정하겠습니다!

@Gilpop8663
Copy link
Collaborator

fe-리뷰완

@chsua chsua merged commit d77dbdb into dev Oct 25, 2023
1 check passed
@woo-chang woo-chang deleted the feat/#816 branch October 26, 2023 07:44
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.

이벤트 코드 삭제 및 UI 버그 수정
3 participants