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

게시글 작성 시 본문 링크 삽입을 하지 않더라도 사용자에게 링크 클릭이 가능하도록 변경 #706

Merged
merged 8 commits into from
Oct 11, 2023

Conversation

Gilpop8663
Copy link
Collaborator

@Gilpop8663 Gilpop8663 commented Oct 4, 2023

🔥 연관 이슈

close: #703

📝 작업 요약

  • 게시글 작성 시 본문 링크 삽입을 하지 않더라도 사용자에게 링크 클릭이 가능하도록 변경

⏰ 소요 시간

2시간

🔎 작업 상세 설명

  • 본문 링크 삽입 시 [[ ]] 을 붙혀주고 있는데, 이걸 작성자에게 붙히도록 하지 않고 렌더링 시 http | https | www 로 시작하는 문자를 정규표현식을 이용하여 [[ 주소 ]]를 붙혀주도록 함
  • 이 때 기존의 사용되던 [[ 링크 ]] 텍스트가 있다면 변환되지 않도록 하였음
  • 현재 홈에서 게시글 본문에 링크가 있다면 a 태그 내부에 a 태그가 있다는 에러가 나는데 본문에서 링크 렌더링 시 span으로 렌더되도록 수정

개선 전

image
image

콘솔에서 key를 설정하라는 에러와 a 태그 내부에 a 태그가 있으면 안된다는 에러

react-jsx-runtime.development.js:87 Warning: Each child in a list should have a unique "key" prop.

Check the render method of `Post`. See https://reactjs.org/link/warning-keys for more information.
react_devtools_backend_compact.js:13096 Warning: validateDOMNesting(...): <a> cannot appear as a descendant of <a>.
    at a

image

개선 후

-.VoTogether.-.Brave.2023-10-04.14-06-22.mp4

에러 개수 0인 모습

image

참고자료

https://regex101.com/r/dVEb0X/1 (정규 표현식 테스트 사이트)

https://teco.chat/chats/1015 (GPT와 페어 프로그래밍)

@github-actions
Copy link

github-actions bot commented Oct 4, 2023

⚡️ Lighthouse report!

Category Score
🟠 Performance 77
🟠 Accessibilty 89
🟢 SEO 100
🟠 PWA 89
Category Score
🟢 First Contentful Paint 0.6 s
🟠 Largest Contentful Paint 2.7 s
🔴 Total Blocking Time 790 ms
🟢 Cumulative Layout Shift 0
🟢 Speed Index 2.8 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.

콘솔에서 key를 설정하라는 에러와 a 태그 내부에 a 태그가 있으면 안된다는 에러

혹시 a 태그 안에 a 태그가 있다는게 어떤 이유 때문인지 아시나요..?🤔🤔

사소한 사용성을 하나씩 개선하시는 점이 멋집니다👍
킵고잉 우스👏

fe-리뷰완

* https?:\/\/는 http:// 혹은 https:// 로 시작하는 지 여부를 확인한다.
* (?!\]\]) 는 뒤에 ]]로 끝나는 지 여부를 확인한다.
* [^\s] 는 공백이 아닌 문자인지 여부를 확인한다.
*/
Copy link
Member

Choose a reason for hiding this comment

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

상세한 주석 정말 좋군요👍

/**
* www.naver.com
* www.tistory.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.

네 기존의 링크가 [[으로 시작하기 때문에 중복해서 [[[[ 기존링크 ]]]]와 같이 변환되지 않게 하기 위해서 [[로 시작한다면 매칭되지 않게 했습니다

Copy link
Collaborator

Choose a reason for hiding this comment

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

세심하시네요


/**
* www.naver.com
* www.tistory.com
Copy link
Member

Choose a reason for hiding this comment

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

혹시 tistory.com 도 링크로 인식되는지 궁금해요!

Copy link
Member

Choose a reason for hiding this comment

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

www.tistory.com
흠 깃허브도 tistory.com 은 링크로 인식이 안되네요 ..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

그러게요! 근데 www. 을 안붙히고 뒤에 .com이나 net을 인식하면 가능은 할 것 같아요

export const convertTextToElement = (text: string) => {
import { convertTextToUrl } from './convertTextToUrl';

export const convertTextToElement = (text: string, isLinkEnabled = true) => {
Copy link
Member

Choose a reason for hiding this comment

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

isLinkEnabled 옵션이 false 이면 링크를 그냥 일반 문자열로 유지되는건가요?🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

네 false라면 span으로 유지되도록 하였습니다

a태그 내부에 a태그가 있는 에러가 이 부분에서 났어서요

홈에서 각각 상세 게시글 링크로 가는 a 태그가 있고 그 내부 배용에 a 태그가 있어서 콘솔 에러가 났던 부분이라서 홈에서만 false로 인자를 받도록 했습니다

Copy link
Collaborator

@chsua chsua left a comment

Choose a reason for hiding this comment

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

링크를 넣으며 불편함을 느꼈다는 점에서
우스가 얼마나 열심히 글을 작성하셨는지 느껴졌습니다 😁

다만 걱정되는 부분이 두 가지있습니다.

  1. 글 등록 이전에는 사용자가 링크가 잘 연결되었는지 알 수 없다.
    • 이전에는 버튼을 누르고 [[]]를 통해 확실한 매뉴얼이 주어졌지만, 지금은 다른 형식 없이 링크가 "자동으로 인식"되어 들어가게 됩니다.
    • 처음 사용하는 사용자는 링크를 붙여넣고 이게 되는지 안되는지 알 수 없어졌다고 생각해요.
    • 일반적으로 글을 쓸 때 link를 넣는 버튼이 존재하거나, 아님 복사했을 때 링크로 연결되었다고 글 작성하는 중에도 구별가능한 UI로 전환해주고 있습니다.
    • 아래 네이버에서는 주소를 복사 붙여넣기하거나, 링크를 입력하고 엔터를 누르면 링크로 인식해서 파란 글씨와 밑줄을 해주고 있습니다.
    • 더불어서 대충 실험해본 결과 앞에 http://www + 끝에 .com 같은 규격이 url인지 확인하는 정규식이 걸려있는 것 같아요.
스크린샷 2023-10-05 오전 11 21 09
  1. 목록에서 링크가 연결이 안되는데, 상세로 들어가면 링크가 연결되는 점
    • 제가 느끼기에 사람들은 목록과 상세페이지에서 가능한 공통된 기능을 누리고 싶어하는 것 같습니다. 상세로 들어가기 전 사진을 보고 싶다고 하는 것처럼 말이에요. 때문에 목록에서 링크 연결을 하지 않는다면 사용자가 불편함을 느낄 수 있지 않을까 생각이 들었어요.
    • 지금도 목록에서도 다른 링크를 들어갈 수 없는데 이 점을 이용하기 보단, 고치는 쪽으로 수정 방향을 잡는건 어떻게 생각하시나요?

우스의 의견이 궁금합니다!
fe-리뷰완

@@ -22,7 +25,7 @@ export const convertTextToElement = (text: string) => {
}

// 링크가 아닌 문자열
return <span>{part}</span>;
return <span key={index}>{part}</span>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

만약 isLinkEnabled가 false라면 모든 문자열은 < span >으로 감싸지게 되는데,

그렇다면 서두에 분기처리해서 early return 해버리는건 어떠세요?

export const convertTextToElement = (text: string, isLinkEnabled = true) => {
  const convertedUrlText = convertTextToUrl(text);

  if (!isLinkEnabled) return  <span >{convertedUrlText.join(""}</span> ;

  ...기존 코드
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

수아가 좋은 제안을 해주셔서 아래와 같이 바꿔볼 수 있었어요 !!👍👍👍

export const convertTextToElement = (text: string, isLinkEnabled = true) => {
  const convertedUrlText = convertTextToUrl(text);
  const linkPattern = /\[\[([^[\]]+)\]\]/g;

  const parts = convertedUrlText.split(linkPattern);

  if (!isLinkEnabled) {
    return parts.join('');
  }


...

/**
* www.naver.com
* www.tistory.com
* (?<!\[\[) 는 앞에 [[로 시작하는 지 여부를 확인한다
Copy link
Collaborator

Choose a reason for hiding this comment

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

세심하시네요


export const convertTextToUrl = (text: string) => {
const httpOrHttpsConvertedText = text.replace(httpsOrHttpRegex, url => `[[${url}]]`);
const wwwConvertedText = httpOrHttpsConvertedText.replace(wwwRegex, url => `[[${url}]]`);
Copy link
Collaborator

Choose a reason for hiding this comment

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

이렇게 되면
[[www.---]] 문자열은 변경되지 않고,
www 문자열은 [[www]]로 변경되는 것 맞나요??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

[[www.---]]와 같이 이미 감싸져 있는 문자열이라면 변경되지 않구요

[[ ]] 으로 감싸져 있지 않은 www라면 [[www.---]]으로 변경됩니당

웹 접근성을 위해 link로 들어가는 태그를 button으로 하여 스페이스바로 진입 가능하도록 함
@Gilpop8663
Copy link
Collaborator Author

  1. 글 등록 이전에는 사용자가 링크가 잘 연결되었는지 알 수 없다.

저도 수아의 제안처럼 textarea에서 파랑색으로 보이면 좋겠다는 생각이 드는데요. 현재 이슈에서 하기엔 코드 변경이 많을 것 같아 다른 이슈를 파고 진행해보려고 합니다. 구현 방법으로는 textarea에서는 a 태그 혹은 해당 글자만 파랑색으로 할수가 없어서 div 태그에서 ContentEditable 속성을 통해서 WYSIWYG 방식으로 진행해야할 것 같아요

image

https://developer.mozilla.org/ko/docs/Web/HTML/Global_attributes/contenteditable

  1. 제가 느끼기에 사람들은 목록과 상세페이지에서 가능한 공통된 기능을 누리고 싶어하는 것 같습니다. 상세로 들어가기 전 사진을 보고 싶다고 하는 것처럼 말이에요. 때문에 목록에서 링크 연결을 하지 않는다면 사용자가 불편함을 느낄 수 있지 않을까 생각이 들었어요.

수아의 제안대로 홈에서도 사이트 링크에 접근이 가능하도록 수정했습니다. a 태그 내부에 a태그가 있는 것을 방지하고자 button으로 DetailLink의 태그를 바꾸었는데요. div를 하지 않고 button을 한 이유는 div에서는 스크린 리더기에서 스페이스바를 눌렀을 때 상세 게시글로 진입이 안되길래 button으로 했습니다

@Gilpop8663
Copy link
Collaborator Author

fe-리뷰요청

Copy link
Collaborator

@chsua chsua left a comment

Choose a reason for hiding this comment

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

너무 늦게 리뷰드려서 죄송합니다!
제가 제안드렸던 부분들에 대해 고민해주셔서 감사합니다!
우스는 늘 먼저 세심하게 많은 부분을 신경써주시는군요!
고생하셨습니다!

@chsua chsua merged commit 89089fb into dev Oct 11, 2023
1 check passed
@woo-chang woo-chang deleted the feat/#703 branch October 14, 2023 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants