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

feat: temporary storage feature for activity report creation using localstorage #1385

Open
wants to merge 12 commits into
base: dev
Choose a base branch
from

Conversation

Davidace136
Copy link
Contributor

요약 *

활동보고서 create 하는 화면에서 종종 로그인 토큰이 만료되어서 튕기는 경우가 있는데, 이 때 작업한 결과물이 모두 날아가는 것을 방지하기 위해서 임시저장 기능을 추가했습니다.

코드에서 그냥 ActivityReportFormData라는 interface를 사용하지 않고 각 element를 쪼개서 작업한 이유 :

  • 기존 ActivityReportFormData는 undefined를 허용하지 않음 (애초에 지금까지 완전히 submit을 하거나 완전히 submit된 데이터를 가져올 때 사용되었던 interface라서 당연히 undefined가 있으면 안됨)
  • 그런데 이번 임시저장은 불러올 때 일부 field들이 undefined로 남아있을 가능성이 높음
  • 따라서 일부 field를 undefined로 유지한 채 ActivityReportFormData를 만드는 것이 불가능하기 때문에 그냥 element 일일이 쪼개서 작업함

It closes #1379

스크린샷

스크린샷 2025-01-25 오전 5 23 55

이후 Task *

  • 없음

@Davidace136 Davidace136 self-assigned this Jan 24, 2025
@Davidace136 Davidace136 linked an issue Jan 24, 2025 that may be closed by this pull request
2 tasks
.npmrc Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

오 이건 뭐지??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아 ㅋㅋㅋ 내 설정 만지다가 실수로 같이 업로드됐네여! 지울게~

Return .npmrc to its original version
@jooyeongmee
Copy link
Contributor

음 뭔가 활보쪽에서만 쓰이지 않고 지원금 쪽이나 나중에 다른 곳에서도 쓰일거 같아서 이걸 매번 각각 그 feature의 코드 파일에 직접 추가하는 것보단 뭔가 추상화해서 밖으로 빼서 common에 넣을 방법이 없을라나..? 흠 나도 좀 더 찾아볼게

@Davidace136 Davidace136 force-pushed the 1379-implement-temporary-storage-feature-for-activity-report branch from 1f80e1b to 55baaf7 Compare January 30, 2025 23:56
@jooyeongmee
Copy link
Contributor

현재 임시저장 관련 사용되는 local storage 함수가 getItem, setItem, removeItem인데 그 중 앞 2개는 각각의 파일로 정의되어 있고 removeItem은 그냥 파일 안에서 직접 호출하는거 같은데 이 3개 함수를 class에 묶어서 class로 사용하면 좋을거 같아
예를 들어 class 이름이 LocalStorageUtil 뭐 이런거라고 하면 LocalStorageUtil.get("key"), LocalStorageUtil.set("key", "value"), LocalStorageUtil.remove("key") 이런 식으로 하면 좋을 거 같은데 가능할까?

@jooyeongmee
Copy link
Contributor

흠 그리고 로컬 스토리지에 이 데이터만 있는게 아니어서 임시저장으로 인해 스토리지에 추가하는거를 일괄로 구분할 수 있게 앞에 뭔가 prefix를 공통으로 붙이고 되게 key를 넣으면 좋을거 같은데 prefix를 뭘 붙이면 좋을진 모르겠어

const getLocalStorage = <T>(name: string) => {
const rawData = localStorage.getItem(name);
if (rawData !== null) {
const storedData = JSON.parse(rawData) as T;
Copy link
Contributor

Choose a reason for hiding this comment

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

혹시나 여기 parse 하다가 에러 날 수도 있으니까 try catch로 감싸는건 어때?


const getLocalStorage = <T>(name: string) => {
const rawData = localStorage.getItem(name);
if (rawData !== null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

괜찮을거 같긴 한데 혹시나 undefined가 들어가게 될 수도 있으니까 null이랑 undefined 다 커버할 수 있는 rawData != null 로 하면 좋을거 같아

const location = watch("location");
const purpose = watch("purpose");
const detail = watch("detail");
const evidence = watch("evidence");
Copy link
Contributor

Choose a reason for hiding this comment

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

뭔가 이거 하나하나 추가할 필요없이 watch() 하면 전체 데이터 다 받아올 수 있을거 같은데?

@@ -82,6 +91,30 @@ const ActivityReportForm: React.FC<ActivityReportFormProps> = ({
endTerm,
});

useEffect(() => {
saveLocalStorage("activity-report", {
Copy link
Contributor

Choose a reason for hiding this comment

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

activity-report는 파일 상단 혹은 constant에 따로 const 로 정의하면 좋을거 같아

@jooyeongmee
Copy link
Contributor

그리고 위에 수정사항들 다 되면 지원금 신청 페이지에도 반영 부탁해요~

@jooyeongmee
Copy link
Contributor

jooyeongmee commented Jan 31, 2025

그리고 가능하면 class에 만료시간 설정 할 수 있는 로직도 추가되면 좋을거 같다
생각해보니 현재 그냥 신청 완료되면 스토리지 removeItem 하는거 같은데 만약에 사용자가 작성만 하고 신청은 안 한채로 페이지 나가버리면 계속 거기 남아있는거니까 만료시간 설정할 수 있게 해서 해당 시간 지나면 자동으로 삭제하는거로
혹시 요거 어려우면 위에꺼 작업하면 내가 이어서 요거 할게 (해보고 싶거나 하고 싶으면 해도 좋고~~)

…al errors & simplify watch() & constant param
@Davidace136 Davidace136 force-pushed the 1379-implement-temporary-storage-feature-for-activity-report branch from eb0e465 to 2dcb123 Compare February 1, 2025 11:51
@jooyeongmee jooyeongmee force-pushed the 1379-implement-temporary-storage-feature-for-activity-report branch 2 times, most recently from 3086d51 to 1f9a86a Compare February 2, 2025 17:07
 into 1379-implement-temporary-storage-feature-for-activity-report
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Temporary Storage Feature For Activity Report
2 participants