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

[Poi-list-elements] POI 리스트에서 스크랩 버튼을 숨길 수 있도록 함 #2990

Merged
merged 6 commits into from
Nov 10, 2023

Conversation

seulgiyoon
Copy link
Contributor

@seulgiyoon seulgiyoon commented Nov 7, 2023

PR 설명

POI 리스트에서 스크랩 버튼을 숨길 수 있도록 합니다. '서울콘' 행사 대응용 작업입니다.

변경 내역

'서울콘' 행사를 통해 컨텐트웹(triple-content-web)의 페이지로 유입된 사용자들은 모두 비로그인 상태이며, 로그인이 필요한 기능을 사용할 수 없습니다. 더불어 트리플앱으로의 연결을 유도하는 루트도 보지 않아야 합니다.

  • 그래서 Triple Document에 포함된 Pois 컴포넌트(POI list) 내 로그인 및 앱과의 연결과 관련된 부분들을 특정한 값을 기준으로 분기하기 위하여, guestMode 값을 받을 수 있도록 추가했습니다.
  • guestModetrue인 경우, POI list의 스크랩 버튼을 숨깁니다.

논의하고 싶은 점

'페이지에서 로그인이 필요한 동작(스크랩, 리뷰쓰기)등이 불가능하며, 앱으로 연결되는 루트에 접근할 수 없음'이라는 기획을 적용하기 위해, 관련 기능을 분기하는 기준 플래그를 guestMode라고 이름붙였습니다.
서울콘 seoulcon 이라고 이름붙일지 고민했으나, Triple Document에 특정 이벤트 이름이 들어가는것이 적절하지 않을 수 있겠다고 생각했어요. 하지만 아직 서울콘 외에 다른 비슷한 이벤트는 없는 상황이라(가능성은 있으나) 명확하게 seoulcon이라고 이름붙이는것이 나을까? 싶기도 합니다. 어떤 이름이 적절할까요?

체크리스트

  • content-web dev QA

스크린샷 & URL

스크린샷 2023-11-07 오후 5 56 22

@seulgiyoon seulgiyoon self-assigned this Nov 7, 2023
@seulgiyoon seulgiyoon added this to the v13.11.0 milestone Nov 7, 2023
@seulgiyoon seulgiyoon marked this pull request as ready for review November 7, 2023 09:03
@seulgiyoon seulgiyoon requested a review from a team as a code owner November 7, 2023 09:03
@seulgiyoon seulgiyoon requested review from zzolain, choisohyun and jhyj0521 and removed request for a team November 7, 2023 09:03
@dongoc
Copy link
Contributor

dongoc commented Nov 7, 2023

아직 서울콘 외에 기능을 제한하는 곳이 없어서 guestMode로 통합하는 건 괜찮은 것 같아요!

다만 서울콘 외에 다른 기획에서 만약 '로그인이 필요한 동작은 가능하지만 앱으로 연결은 불가능'같은 경우의 수가 생긴다면 guestMode보다는 appRouteInaccessible, authRouteInaccessible (변수명은 대애충 지었습니다..) 같이 기능별로 나누는 것도 괜찮을 것 같습니다.
서울콘이라는 이름으로는 상위에서 컨트롤할 수 있게요!

const ACCESS_POLICY = {
  seoulCon:{
    appRouteInaccessible: true,
    authRouteInaccessible: true
  },
  extraEvent: {
    appRouteInaccessible: true,
    authRouteInaccessible: false
  }
}

근데 지금은 서울콘 하나라서 지금은 guestMode로 가고 나중에 기능 구분이 필요할 때 나누는 게 더 좋을 것 같다는 의견입니당!

dongoc
dongoc previously approved these changes Nov 8, 2023
zhsks528
zhsks528 previously approved these changes Nov 9, 2023
- '서울콘' 행사 대응용
- '서울콘' 행사를 통해 triple-content-web의 페이지로 유입된 사용자들은 모두 비로그인 상태이며, 로그인이 필요한 기능을 사용할 수 없음. 더불어 트리플앱으로의 연결을 유도하는 루트도 보지 않아야 함
- Triple Document로 제공되는 가이드와 POI 영역에서 로그인 및 앱과의 연결과 관련된 부분들을 guestMode라는 값을 기준으로 분기하기 위하여 값을 추가(예 : guestMode가 true인 경우, Pois 컴포넌트의 스크랩 버튼 숨기기)
- extended-poi-list-element는 Pois 엘리먼트에서 사용되지 않아 적용제외
@seulgiyoon seulgiyoon dismissed stale reviews from zhsks528 and dongoc via b8cb4a2 November 10, 2023 00:45
@seulgiyoon seulgiyoon force-pushed the feat/hide-scrap-poi-list branch from b5ac589 to b8cb4a2 Compare November 10, 2023 00:45
Copy link

codecov bot commented Nov 10, 2023

Codecov Report

Attention: 24 lines in your changes are missing coverage. Please review.

Comparison is base (318b500) 12.10% compared to head (b8cb4a2) 12.09%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2990      +/-   ##
==========================================
- Coverage   12.10%   12.09%   -0.01%     
==========================================
  Files         690      691       +1     
  Lines       36444    36451       +7     
  Branches     8994     8996       +2     
==========================================
  Hits         4410     4410              
- Misses      32027    32034       +7     
  Partials        7        7              
Files Coverage Δ
packages/triple-document/src/elements/pois.tsx 0.00% <0.00%> (ø)
...ackages/poi-list-elements/src/carousel-element.tsx 0.00% <0.00%> (ø)
...ges/triple-document/src/prop-context/guest-mode.ts 0.00% <0.00%> (ø)
...poi-list-elements/src/compact-poi-list-element.tsx 0.00% <0.00%> (ø)
packages/triple-document/src/triple-document.tsx 0.00% <0.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@seulgiyoon seulgiyoon merged commit 80f0a3c into main Nov 10, 2023
7 checks passed
@seulgiyoon seulgiyoon deleted the feat/hide-scrap-poi-list branch November 10, 2023 00:54
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.

5 participants