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

[FIX] 네이버 검색 결과 태그 제거, 카테고리 대분류 추출 #86

Merged
merged 7 commits into from
Dec 5, 2024
Merged

[FIX] 네이버 검색 결과 태그 제거, 카테고리 대분류 추출 #86

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4eb9873
fix : remove tags and extract primary category only
keemsebin Dec 4, 2024
b53ab1c
style: adjust font-weight
keemsebin Dec 4, 2024
52321d5
chore: Remove unused folders
keemsebin Dec 4, 2024
a5a8f3b
fix : add trim func
keemsebin Dec 4, 2024
a38ff49
fix: resolve text css conflict by explicitly setting font size
keemsebin Dec 4, 2024
c530a9c
chore : remove unused import
keemsebin Dec 4, 2024
2eeec62
feat: add autoFocus to Input component
keemsebin Dec 4, 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
Empty file removed src/apis/.gitkeep
View file
Open in desktop
Empty file.
6 changes: 3 additions & 3 deletions src/components/common/Input/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { forwardRef } from 'react';
import { Props } from './Input.types';

import { Icon } from '../Icon';
import { Caption } from '../Typography';
import { Body4 } from '../Typography';

export const Input = forwardRef<HTMLInputElement, Props>(
({ value, onClickReset, isValid, errorMessage, ...props }, ref) => {
Expand All @@ -29,9 +29,9 @@ export const Input = forwardRef<HTMLInputElement, Props>(
)}
</div>
{helperMessage && (
<Caption className="text-primary mt-1">
<Body4 className="text-[14px] text-primary mt-1">
{errorMessage || '유투브 링크 혹은 지도 링크만 가능합니다.'}
</Caption>
</Body4>
Comment on lines +32 to +34
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Typography 스타일이 중복되어 있습니다

Body4 컴포넌트는 이미 text-body4 클래스를 통해 14px 폰트 크기를 정의하고 있습니다. Tailwind 설정에서 text-body4가 14px로 정의되어 있으므로, text-[14px] 클래스는 불필요한 중복입니다. 해당 클래스를 제거하여 Typography 시스템의 일관성을 유지하시기 바랍니다.

🔗 Analysis chain

Typography 스타일 중복 확인 필요

Body4 컴포넌트가 이미 기본 폰트 크기를 정의하고 있다면, text-[14px] 클래스는 중복될 수 있습니다. Typography 시스템의 일관성을 위해 확인이 필요합니다.

Let me check the Tailwind configuration to see if text-body4 class defines a specific font size.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if Body4 component already defines font size
rg -A 5 "Body4" src/components/common/Typography

Length of output: 2007

Script:

#!/bin/bash
# Check Tailwind configuration for text-body4 class definition
fd -e js -e ts "tailwind.config" --exec cat {}

Length of output: 785

)}
</div>
);
Expand Down
1 change: 1 addition & 0 deletions src/components/features/LinkForm/LinkInput.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export const LinkInput = ({ onNext, onHomeClick, context }: LinkInputProp<string
<div className="w-full flex flex-col items-start gap-6 my-36 px-6">
<Body1>{`아래에 링크를 입력해주시면,\n특별한 장소 정보를 추출해드릴게요.`}</Body1>
<Input
autoFocus
value={inputValue}
onChange={onChange}
onBlur={onBlur}
Expand Down
2 changes: 1 addition & 1 deletion src/hooks/common/useInput.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export const useInput = (initialValue = '', validation: string[] = ['https', 'ww

const onChange: ChangeEventHandler<HTMLInputElement> = useCallback((e) => {
const value = e.target.value;
setState(value);
setState(value.trim());
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

입력값 트리밍 시점 재고가 필요합니다

onChange 이벤트에서 즉시 트리밍하는 것은 사용자 경험을 해칠 수 있습니다. 사용자가 입력 중에 공백을 사용하는 것이 불가능해집니다.

다음과 같이 onBlur 이벤트에서 트리밍하는 것을 추천드립니다:

-    setState(value.trim());
+    setState(value);

그리고 onBlur 함수에서 트리밍을 수행:

   const onBlur = useCallback(() => {
+    setState(state.trim());
     if (state.length > 0) {
       setIsValid(validation.some((criteria) => state.includes(criteria)));
     }
   }, [state, validation]);

Committable suggestion skipped: line range outside the PR's diff.

}, []);

const onBlur = useCallback(() => {
Expand Down
3 changes: 2 additions & 1 deletion src/pages/MapView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { useMapState } from '@/hooks/common/useMapState';
import { useMarkers } from '@/hooks/common/useMarkers';
import { useSessionDataLoader } from '@/hooks/common/useSessionDataLoader';
import { Place } from '@/types/naver';
import { parseSearchResult } from '@/utils/parseSearchResult';

export const MapView = () => {
const { token } = useAuth();
Expand Down Expand Up @@ -54,7 +55,7 @@ export const MapView = () => {
resetCurrentLocation();
const result = await refetch();
if (result?.data?.items) {
handleDataUpdate(result.data.items, 'search');
handleDataUpdate(parseSearchResult(result.data.items), 'search');
}
};

Expand Down
2 changes: 1 addition & 1 deletion src/utils/CustomMarker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export const CustomMarker = ({
<object data="${markerIcon}" type="image/svg+xml" style="width: 37px; height: 47px;"></object>
<object data="${categoryIcon}" type="image/svg+xml" style="width: 33px; height: 33px; position: absolute; left: 50%; top: 45%; transform: translate(-50%, -50%);"></object>
</button>
<span style="display: block; font-size: 12px; font-weight: 400; color: black; text-align: center; white-space: normal; margin-top: 4px;">
<span style="display: block; font-size: 12px; font-weight: 600; color: black; text-align: center; white-space: normal; margin-top: 4px;">
${shouldShowTitle ? title : ''}
</span>
Comment on lines +45 to 47
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

보안 취약점 개선이 필요합니다

문자열 템플릿을 통한 직접적인 HTML 삽입은 XSS 공격에 취약할 수 있습니다.

다음과 같이 HTML 이스케이프 함수를 추가하고 적용하는 것을 추천드립니다:

+const escapeHtml = (str: string) => {
+  return str
+    .replace(/&/g, '&amp;')
+    .replace(/</g, '&lt;')
+    .replace(/>/g, '&gt;')
+    .replace(/"/g, '&quot;')
+    .replace(/'/g, '&#039;');
+};

   <span style="display: block; font-size: 12px; font-weight: 600; color: black; text-align: center; white-space: normal; margin-top: 4px;">
-    ${shouldShowTitle ? title : ''}
+    ${shouldShowTitle ? escapeHtml(title) : ''}
   </span>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<span style="display: block; font-size: 12px; font-weight: 600; color: black; text-align: center; white-space: normal; margin-top: 4px;">
${shouldShowTitle ? title : ''}
</span>
const escapeHtml = (str: string) => {
return str
.replace(/&/g, '&amp;')
.replace(/</g, '&lt;')
.replace(/>/g, '&gt;')
.replace(/"/g, '&quot;')
.replace(/'/g, '&#039;');
};
<span style="display: block; font-size: 12px; font-weight: 600; color: black; text-align: center; white-space: normal; margin-top: 4px;">
${shouldShowTitle ? escapeHtml(title) : ''}
</span>

</div>
Expand Down
17 changes: 17 additions & 0 deletions src/utils/parseSearchResult.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { Place } from '@/types/naver';

const removeHtmlTags = (result: string) => {
return result.replace(/<\/?[^>]+(>|$)/g, '');
};

const extractCategory = (category: string) => {
return category.split('>')[0];
};
Comment on lines +7 to +9
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

카테고리 추출 시 예외 처리가 필요합니다

빈 문자열이나 구분자가 없는 경우에 대한 처리가 없습니다.

다음과 같이 예외 처리를 추가하는 것을 추천드립니다:

-  return category.split('>')[0];
+  return category?.split('>')?.[0]?.trim() ?? category ?? '';
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const extractCategory = (category: string) => {
return category.split('>')[0];
};
const extractCategory = (category: string) => {
return category?.split('>')?.[0]?.trim() ?? category ?? '';
};


export const parseSearchResult = (data: Place[]) => {
return data.map((item) => ({
...item,
title: removeHtmlTags(item.title),
category: typeof item.category === 'string' ? extractCategory(item.category) : '',
}));
};
Comment on lines +11 to +17
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

타입 안전성 강화가 필요합니다

Place 타입의 필수 필드에 대한 검증이 부족합니다.

다음과 같이 타입 가드와 필수 필드 검증을 추가하는 것을 추천드립니다:

 export const parseSearchResult = (data: Place[]) => {
+  if (!Array.isArray(data)) return [];
   return data.map((item) => ({
     ...item,
-    title: removeHtmlTags(item.title),
+    title: item.title ? removeHtmlTags(item.title) : '',
     category: typeof item.category === 'string' ? extractCategory(item.category) : '',
   }));
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const parseSearchResult = (data: Place[]) => {
return data.map((item) => ({
...item,
title: removeHtmlTags(item.title),
category: typeof item.category === 'string' ? extractCategory(item.category) : '',
}));
};
export const parseSearchResult = (data: Place[]) => {
if (!Array.isArray(data)) return [];
return data.map((item) => ({
...item,
title: item.title ? removeHtmlTags(item.title) : '',
category: typeof item.category === 'string' ? extractCategory(item.category) : '',
}));
};

Loading