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

[FE] issue45: 필터 기능 구현 #68

Merged

Conversation

nan-noo
Copy link
Collaborator

@nan-noo nan-noo commented Jul 16, 2022

요약

메인페이지 필터 기능 구현

세부사항

  • 필터 다중 선택 - checkbox로 구현
  • 필터 좌우 슬라이드 구현 - 끝까지 도달시 반대방향 끝으로 돌아가도록 구현 + 반응형, sticky
  • 메인 페이지 로직 변경 - 스터디 검색 및 조회 api 하나로 데이터 불러오도록 수정 + 타입 수정
  • 메인페이지를 감싸던 Wrapper를 필터 섹션 제외한 스터디카드리스트만 감싸도록 변경 - 필터 스타일 때문

close #45

nan-noo and others added 30 commits July 13, 2022 15:13
svg에 색이 변하지 않는 이유 - stroke 대신 fill이 필요한 경우가 있었고, stroke의 width가 0이어서 변하지 않는 경우도 있었음
useQuery의 key에서 selectedFilters를 제거해서 해결

+ 드롭박스가 카드에 가려지는 오류 수정 -> z-index 스타일 추가
스터디 조회와 검색 기능이 합쳐져서 msw 로직 수정 및 필요없는 api 함수 제거 + 스터디 key 수정(description -> excerpt)
슬라이드 버튼 클릭시 좌우 스크롤 + sticky 적용
끝까지 이동했을 때 한 번 더 클릭시 스크롤 반대 방향 끝으로 이동
@nan-noo nan-noo added 🚀 feature New feature or request 😁 frontend New frontend feature labels Jul 16, 2022
@nan-noo nan-noo requested a review from airman5573 July 16, 2022 17:36
@nan-noo nan-noo self-assigned this Jul 16, 2022
@nan-noo nan-noo linked an issue Jul 16, 2022 that may be closed by this pull request
export const Default = Template.bind({});
Default.args = {
shortTitle: 'JS',
description: '자바스크립트',
Copy link
Collaborator

Choose a reason for hiding this comment

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

shortTitle -> shortName / description -> fullName이 좋을것 같아요. description은 어떤 장황한 설명이 있을때 더 어울린다고 생각합니다.

const filterParams = selectedFilters.map(({ id, categoryName }) => `&${categoryName}=${id}`).join('');

const response = await axiosInstance.get(
`/api/studies/search?page=${page}&size=${size}&title=${title}&${filterParams}`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

title이 없는 경우에는 path에 넣지 않는게 좋을것 같아요

import { css } from '@emotion/react';
import styled from '@emotion/styled';

interface CheckBoxLabelProps {
Copy link
Collaborator

Choose a reason for hiding this comment

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

interface vs type
비교하고 상의해서 하나로 일관성있게 가는걸로~
https://stackoverflow.com/questions/49562569/typed-react-props-as-type-or-an-interface

flex-direction: column;
align-items: center;
justify-content: flex-end;
gap: 8px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

row, column 갭이 모두 필요한가요?

shortName: string;
fullName: string;
isChecked: boolean;
handleFilterButtonClick: React.ChangeEventHandler<HTMLInputElement>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

HTMLInputElement인가요? 버튼 아닌가요?

if (!title) {
return res(ctx.status(400), ctx.json({ message: '검색어가 비어있습니다' }));
}
const generation = req.url.searchParams.getAll('generation');
Copy link
Collaborator

Choose a reason for hiding this comment

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

배열로 받으니까 복수 형태로 쓰는게 좋을것 같아요!

position: sticky;
top: 85px;
max-width: 1280px;
height: fit-content;
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.

fit-content 말하신 건가요?? 삭제했습니다 ㅎㅎ

}
`;

export const FilterSectionHeader = styled.h2`
Copy link
Collaborator

Choose a reason for hiding this comment

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

필요한 컴포넌트인가요? 왜 항상 display: none인가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

section 태그를 사용할 때는 일반적으로 헤더 태그가 필요하다고 생각해서 넣었는데, 제목이 필요한 상황에서만 넣으면 되는 것 같네요..! 우선 제거했을 때 접근성에는 문제가 없어 보입니다. mdn 문서에 should always라고 써져 있긴 합니당
stackoverflow에는 항상 필요하진 않다고 되어 있네요.

const tagFilters = filterByCategory(data?.filters, 3);

const handleLeftSlideButtonClick = () => {
if (!sliderRef.current) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (!sliderRef.current) {
if (!sliderRef.current) return

이쪽으로 맞추는건 어떨까요

thumbnailUrl={study.thumbnail}
thumbnailAlt={`${study.title} 스터디 이미지`}
title={study.title}
description={study.excerpt}
Copy link
Collaborator

Choose a reason for hiding this comment

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

excerpt!

@airman5573 airman5573 merged commit 01dcadf into woowacourse-teams:develop Jul 18, 2022
@nan-noo nan-noo deleted the feat/45-filter-with-category branch July 26, 2022 04:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚀 feature New feature or request 😁 frontend New frontend feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FE] 필터 기능 구현 [FE] 컴포넌트 이름 수정 [FE] 공통 컴포넌트 폴더 분리
3 participants