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

#374 Refactor: 회비 페이지 리팩토링 #388

Merged
merged 15 commits into from
Mar 7, 2025

Conversation

JIN921
Copy link
Collaborator

@JIN921 JIN921 commented Feb 26, 2025

1. 무슨 이유로 코드를 변경했나요?


2. 어떤 위험이나 장애를 발견했나요?

  • 문제 1: 회비 페이지 문제
    회비 리스트 css 문제..

Image
이렇게 보이는 문제를 해결 하면
Image
이렇게 되는 정말 안타까운 상황 발생...

https://github.com/user-attachments/assets/33e36bb7-d3f2-4f7a-ae85-e95c1b17df8f
그래서 일단 화면은 고정하고 회비 리스트 내부에서 스크롤 가능하도록 변경,,, 유저들이 잘 알지는 몰르겟습니다.. 의논 필요

  • 문제 2: 영수증 pdf 미리보기 문제
    ifram으로 간단 해결하고자 햇지만.. 이거 쓰면... pdf 파일이 새로고침할 때마다 다운로드 되는 이상한 문제 발생... 짱남..
    그리고 미리보기에 사진 안 뜸 ㅠ ㅠ

그래서 이것 저것 해보다가 결국,,, pdf.worker.js 파일을 로컬에 저장하고 직접 로드하도록 변경하고, react-pdf와 pdfjs-dist 버전 맞도록 잘... 잘 설치 해서 해결 햇습니다 해피,, ㅎ
image
image

진짜 영수증에 이렇게 힘을 쏟고 싶지 않앗는데.. 해결하고 싶은 마음에 어뜨케든 햇내요..

3. 완료 사항

  • 회비 코드 리팩토링
  • 영수증 안 보이는 문제 해결
  • pdf 로드 해결

4. 추가 사항

pdf 미리보기 할 일 있음 저에게 말해주세요^_^

default.mp4

수정사항 적용했습니다!

@JIN921 JIN921 added 🐞 Fix Something isn't working 🎨 Design CSS 스타일링 labels Feb 26, 2025
@JIN921 JIN921 requested a review from yezzan9 February 26, 2025 15:51
@JIN921 JIN921 self-assigned this Feb 26, 2025
@JIN921 JIN921 linked an issue Feb 26, 2025 that may be closed by this pull request
3 tasks
@yezzan9
Copy link
Member

yezzan9 commented Feb 27, 2025

문제 1: 회비 페이지 문제
회비 리스트 css 문제..

이거 전에 멤버 페이지도 동일한 스타일이었는데, 하단까지 전부 채우는 것이 아닌, 존재하는 만큼만 배경색을 추가하도록 디자인을 수정했었어요! 회의 때 얘기해보면 좋을 것 같습니당

_.mp4

그래서 일단 화면은 고정하고 회비 리스트 내부에서 스크롤 가능하도록 변경,,, 유저들이 잘 알지는 몰르겟습니다.. 의논 필요

이게 pc로 볼 땐 괜찮은데, 모바일처럼 기기의 뷰포트 height가 작아지면 스크롤 영역이 겹치게 되지 않을까요? (전체 페이지에 대한 스크롤, 회비 사용 영역에 대한 스크롤)

Copy link
Member

@yezzan9 yezzan9 Feb 27, 2025

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.

넵 다 지우고 다시 넣은 것이에요,, 근데 pdf.js 폴더에서 저 mjs 파일을 다 쓰는 건 아닌 거 같아서 다시 확인해 보고 지울게요 지웟다가 안 되면 revert 하면 되니까! 하핫

Comment on lines 51 to 60
<div
style={{
width: '100%',
height: 'auto',
display: 'flex',
justifyContent: 'center',
cursor: 'pointer',
}}
onClick={openPdfInNewTab}
>
Copy link
Member

Choose a reason for hiding this comment

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

어차피 스타일링을 추가해야하는 태그라면, 스타일드 컴포넌트로 정의하고 eslint 규칙을 비활성화하는 주석을 제거하면 좋을 것 같아요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ㅜㅜ 사실 알앗지만.. 하다가 힘들어서 그냥 둿어요.. 고칠게욤

Comment on lines 72 to 84
<button
type="button"
onClick={onRequestClose}
style={{
marginTop: '10px',
padding: '5px 10px',
borderRadius: '5px',
border: 'none',
backgroundColor: `${theme.color.negative}`,
color: `${theme.color.gray[100]}`,
cursor: 'pointer',
}}
>
Copy link
Member

Choose a reason for hiding this comment

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

위와 같습니다!

Comment on lines 42 to 43
const isPdfFile = (url: string) => url.toLowerCase().endsWith('.pdf');

Copy link
Member

Choose a reason for hiding this comment

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

함수명이 isPdfUrl이면 매개변수로 전달하는 값이 url인 것을 더 명확하게 알 수 있을 것 같아요!

@yezzan9
Copy link
Member

yezzan9 commented Feb 27, 2025

image

width에 맞추어서 확대 및 채우기로 구현하는 건 어려울까요?

@yezzan9
Copy link
Member

yezzan9 commented Feb 27, 2025

image

여기 위에 t라고 뜨는 건 뭔가요?!
그리고 서비스에 맞게 모달 배경색이 회색이거나, 투명한 배경이면 좋을 것 같아요 (투명이라면 닫기 버튼 대신 x를 추가하는 식으로?)

@JIN921
Copy link
Collaborator Author

JIN921 commented Feb 27, 2025

문제 1: 회비 페이지 문제
회비 리스트 css 문제..

이거 전에 멤버 페이지도 동일한 스타일이었는데, 하단까지 전부 채우는 것이 아닌, 존재하는 만큼만 배경색을 추가하도록 디자인을 수정했었어요! 회의 때 얘기해보면 좋을 것 같습니당

_.mp4

그래서 일단 화면은 고정하고 회비 리스트 내부에서 스크롤 가능하도록 변경,,, 유저들이 잘 알지는 몰르겟습니다.. 의논 필요

이게 pc로 볼 땐 괜찮은데, 모바일처럼 기기의 뷰포트 height가 작아지면 스크롤 영역이 겹치게 되지 않을까요? (전체 페이지에 대한 스크롤, 회비 사용 영역에 대한 스크롤)

image
아하 이런식으로요?? 괜찮을 듯 합니다 이게 나을 듯 해욥

@JIN921
Copy link
Collaborator Author

JIN921 commented Feb 27, 2025

image

여기 위에 t라고 뜨는 건 뭔가요?! 그리고 서비스에 맞게 모달 배경색이 회색이거나, 투명한 배경이면 좋을 것 같아요 (투명이라면 닫기 버튼 대신 x를 추가하는 식으로?)

t가 먼가 햇네요 ㅋㅋㅋ 저거 그냥 캡처할 때 주소 같이 뜬 것 입니다 암것두 아님!! 모달.. 수정해 볼게욥..

@JIN921
Copy link
Collaborator Author

JIN921 commented Feb 27, 2025

image

width에 맞추어서 확대 및 채우기로 구현하는 건 어려울까요?

하 이게....... pdf가 맘대로 안 돼서... 일단 저거 건들 때마다 눈물 좀 흘려야 돼서 일단 둿는데.. 더 시도를 해볼게욥..

@yezzan9
Copy link
Member

yezzan9 commented Feb 28, 2025

image 아하 이런식으로요?? 괜찮을 듯 합니다 이게 나을 듯 해욥

넹 맞아요!!👍🏻

@JIN921 JIN921 requested a review from yezzan9 March 5, 2025 01:17
@JIN921 JIN921 changed the title Refactor#374/회비 페이지 리팩토링 #374 Refactor: 회비 페이지 리팩토링 Mar 6, 2025
Comment on lines 2 to 12
import ReactModal from 'react-modal';
import * as S from '@/styles/receipt/ReceiptMain.styled';
import styled from 'styled-components';

interface ReceiptModalProps {
isOpen: boolean;
selectedImage: string;
onRequestClose: () => void;
}

const StyledModal = styled(ReactModal)`
Copy link
Member

Choose a reason for hiding this comment

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

pdf 모달과 import명 통일해주세요!

@@ -1,9 +1,9 @@
/* eslint-disable jsx-a11y/no-static-element-interactions */
/* eslint-disable jsx-a11y/click-events-have-key-events */
import React from 'react';
import Modal from 'react-modal';
Copy link
Member

Choose a reason for hiding this comment

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

😂

@JIN921 JIN921 requested a review from yezzan9 March 6, 2025 13:57
@JIN921 JIN921 merged commit 7aacfa2 into develop Mar 7, 2025
@JIN921 JIN921 deleted the refactor#374/회비-페이지-리팩토링 branch March 7, 2025 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎨 Design CSS 스타일링 🐞 Fix Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor: 회비 페이지 리팩토링
2 participants