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.

Sign up for GitHub

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

Jump to bottom

fix: 사이트 이름 클릭 시 사이트로 이동하게 수정 #293

Closed
wants to merge 5 commits into from
Closed

fix: 사이트 이름 클릭 시 사이트로 이동하게 수정 #293

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
654becb
feat: 링크의 상위 도메인 추출하는 함수 추가
ssolfa Aug 28, 2024
7d5b24c
feat: 도메인으로 링크 여는 부분 추가
ssolfa Aug 28, 2024
4ceda5f
Merge branch 'develop' into fix/#266-search-result-link-click-redirect
ssolfa Aug 28, 2024
f48abcb
refactor: 상위 도메인 추출함수를 utils로 분리 및 ResultList에 적용
ssolfa Aug 28, 2024
ef48e1b
refactor: window.open()에서 기본값인 _blank 제거
ssolfa Aug 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/search/components/ResultList/ResultList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,22 @@ export const ResultList = () => {
const { data: currentUser } = useGetUserData();
const userId = currentUser?.email || '';

const handleExtractDomain = (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.

리액트에서 handle~로 시작하는 네이밍은 이벤트 핸들러에 사용되는 것이 일반적이니 handle을 제거하고 함수가 하는 일을 명확하게 드러낼 수 있도록 extractOrigin 정도로 함수 이름을 바꾸는 것이 어떨까요?

또 리액트 의존성 없이 범용적으로 사용할 수 있는 함수이니 위치를 utils 폴더 아래로 옮겨도 좋을 것 같네요

Copy link
Collaborator

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.

오... 👍👍👍👍👍 적극반영 하였습니다!!!

refactor: 상위 도메인 추출함수를 utils로 분리 및 ResultList에 적용

try {
const { origin } = new URL(url);
return origin;
} catch (error) {
return url;
}
};

return (
<NoResultFallback results={data?.pages[0].resultList} fallback={<NoResult />}>
<Suspense fallback={<Loading />}>
{data?.pages.map((page) => {
return page.resultList.map((item, itemIndex) => {
const isLastItem = page.resultList.length === itemIndex + 1;
const domain = handleExtractDomain(item.link);
return (
<div key={item.id}>
<LogClick
Expand All @@ -59,6 +69,7 @@ export const ResultList = () => {
>
<ResultListItem
{...item}
domain={domain}
onClick={() => window.open(item.link)}
ref={isLastItem ? lastElementRef : null}
/>
Expand Down
11 changes: 11 additions & 0 deletions src/search/components/ResultList/ResultListItem.style.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,17 @@ export const StyledLinkImage = styled.img`
export const StyledLinkTitle = styled.span`
${(props) => props.theme.typo.body1}
color: ${(props) => props.theme.color.textSecondary};
text-decoration: none;

&:hover {
text-decoration: underline;
}
`;

export const StyledDomain = styled.div`
cursor: pointer;
display: flex;
align-items: center;
`;

export const StyledDate = styled.span`
Expand Down
25 changes: 18 additions & 7 deletions src/search/components/ResultList/ResultListItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,37 @@ import {
StyledTitle,
StyledContentWrap,
StyledInformationWrap,
StyledDomain,
} from './ResultListItem.style';

interface ResultListItemProps
extends Pick<React.ComponentProps<'div'>, 'onClick'>,
ResultListItemResponse {}
ResultListItemResponse {
domain?: string;
}

export const ResultListItem = forwardRef<HTMLDivElement, ResultListItemProps>(
({ title, content, date, thumbnail, favicon, source, onClick }, ref) => {
({ title, content, date, thumbnail, favicon, source, domain, onClick }, ref) => {
const isVerticalLayout = thumbnail.length >= RESULT_LIST_ITEM_THUMBNAIL_LENGTH;
const isHorizontalLayout = !isVerticalLayout && thumbnail.length > 0;
const handleDomainClick = (e: React.MouseEvent) => {
e.stopPropagation();
if (domain) {
window.open(domain, '_blank');
Copy link
Member

Choose a reason for hiding this comment

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

window.open의 target 속성은 기본값이 _blank라네요

#289 (comment)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

사실 저 pr 보고 호버 썼는데... 리뷰를 안봤네요,,,,,,,,,,,,,,,,, 수정했습니다!!

refactor: window.open()에서 기본값인 _blank 제거

}
};

return (
<StyledResultListItem ref={ref} onClick={onClick}>
<StyledContentWrap $isHorizontalLayout={isHorizontalLayout}>
<StyledInformationWrap>
<StyledLinkImageWrap>
<StyledLinkImage src={favicon || '/'} alt="favicon" onError={onErrorImg} />
</StyledLinkImageWrap>
<Spacing direction="horizontal" size={4} />
<StyledLinkTitle>{source}</StyledLinkTitle>
<StyledDomain onClick={handleDomainClick}>
<StyledLinkImageWrap>
<StyledLinkImage src={favicon || '/'} alt="favicon" onError={onErrorImg} />
</StyledLinkImageWrap>
<Spacing direction="horizontal" size={4} />
<StyledLinkTitle>{source}</StyledLinkTitle>
</StyledDomain>
<Spacing direction="horizontal" size={4} />
<StyledDate>·</StyledDate>
<Spacing direction="horizontal" size={4} />
Expand Down