-
Notifications
You must be signed in to change notification settings - Fork 1
[25.08.13 / TASK-244] Refactor - 일부 뉴스레터 레이아웃 오류 해결 #42
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
Conversation
Walkthroughinsights 템플릿 3종의 마크업을 정리했습니다. 외부 링크에 target/rel 속성을 추가했고, 썸네일과 텍스트 레이아웃을 단순화했으며, index 푸터를 경량 레이아웃으로 교체했습니다. user_weekly_trend에서 신규 게시글 문구의 표시 조건을 문장 내부로 이동했습니다. Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (2)
templates/insights/index.html (1)
107-113
: 푸터 아이콘: alt 텍스트 누락 및 Base64 이미지 용량 이슈
- 접근성: 장식 이미지라면
alt=""
를 명시해 스크린리더가 건너뛰도록 권장합니다.- 용량: 대용량 Base64 인라인 이미지는 메일 본문 크기를 크게 늘려 전달/로딩에 악영향을 줄 수 있습니다. 더 가벼운 자산(용량 축소, 단색/문자 로고, 혹은 이미지 제거) 사용을 검토하세요.
최소한 아래 수정은 반영 바랍니다.
- <img style="display: table-cell; width: 20px; margin-right: 10px; height: 20px; flex-shrink: 0; border-radius: 2px;" src="data:image/png;base64,..." /> + <img alt="" style="display: table-cell; width: 20px; margin-right: 10px; height: 20px; flex-shrink: 0; border-radius: 2px;" src="data:image/png;base64,..." />templates/insights/user_weekly_trend.html (1)
80-84
: 외부 링크 보안 속성 추가는 적절하나, 동일 섹션의 다른 링크에도 일관 적용 필요아래 else 분기(라인 80
84)에는69)의 동일 링크에는 아직 미적용입니다. 보안/일관성을 위해 동일 속성 적용을 권장합니다.target
/rel
이 추가되어 있지만, 상단 days_ago 분기(라인 62다음과 같이 수정을 제안합니다(변경 범위 바깥이므로 참고 스니펫 제공):
<a href="https://velog-dashboard.kro.kr/leaderboards?based=post&sort=viewCount&limit=10&utm_source=email&utm_medium=weekly_analysis" style="box-sizing: border-box; color: #999999; font-weight: bold; text-decoration: underline" target="_blank" rel="noopener noreferrer" > 리더보드 </a>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
templates/insights/index.html
(3 hunks)templates/insights/user_weekly_trend.html
(3 hunks)templates/insights/weekly_trend.html
(1 hunks)
🔇 Additional comments (5)
templates/insights/weekly_trend.html (1)
25-25
: 외부 링크 보안 속성 추가 LGTM
rel="noopener noreferrer"
추가로 탭 간 참조 차단이 되어 보안 면에서 적절합니다.target="_blank"
와의 조합도 적합합니다.templates/insights/index.html (2)
50-52
: 외부 링크에 target/rel 속성 일괄 적용: 보안/UX 측면에서 적절합니다CTA/토큰 갱신/푸터 링크에
target="_blank"
와rel="noopener noreferrer"
적용이 일관되고 적절합니다.Also applies to: 89-91, 117-118, 126-127, 135-136
42-42
: 날짜 구간 마크업 보정 LGTM구문 보정으로 문장/마크업 안정성이 좋아졌습니다.
templates/insights/user_weekly_trend.html (2)
18-25
: 신규 게시글 문구 표시 조건 내부화: 의도에 부합하며 중복 조건 제거에 유효합니다조건을 문장 내부로 옮겨 불필요한 분기/여백을 줄인 점 좋습니다. 렌더링/가독성 모두 개선됩니다.
110-113
: 게시글 타이틀 링크 보안 속성 추가 LGTM
target="_blank"
+rel="noopener noreferrer"
조합 적용이 잘 되었습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다! 🔥 👍 테스트시 잘 되어서 큰 문제는 없을 것 같아요!
다만 스타일적으로 사소한 부분들이 조금 거슬려서, 이참에 수정해주실 수 있을까요?? 😅😅🙏
- 슬랙으로 말씀드렸듯이 리마인더 본문이 다소 옅음
- 언더라인 들어가는 글씨들에 공백이 포함됨
- 공통 트렌드엔 태그 이모지(📌)가 있으나 개인 트렌드엔 없음
한 번 살펴봐주세요~!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
templates/insights/user_weekly_trend.html (2)
99-105
: 이메일 클라이언트 호환성: flex 레이아웃은 Outlook 등에서 깨질 수 있음카드 내부 컨테이너에 display:flex를 사용하면 Outlook(Windows) 등 일부 메일 클라이언트에서 레이아웃이 무너질 수 있습니다. 핵심 섹션(썸네일+텍스트 병렬 배치)은 테이블 기반(또는 하이브리드: 테이블+MSO 조건부 주석)으로 전환을 고려해 주세요. 주요 클라이언트에서 렌더링 테스트 권장.
원하시면 해당 블록을 테이블 기반으로 변환한 최소 변경안(썸네일 100px 고정, 텍스트 유동)을 제안해 드릴 수 있습니다.
101-105
: 썸네일 호환성 개선: width/height 속성 및 display:block 추가 권장여전히 object-fit을 사용하고 width/height 어트리뷰트가 없습니다. 많은 이메일 클라이언트에서 object-fit 미지원으로 이미지가 찌그러질 수 있으므로 명시적 width/height와 display:block을 추가해 주십시오.
적용 제안(diff):
- <img - src="{{item.thumbnail}}" - alt="{{item.title}}" - style="box-sizing: border-box; width: 100px; height: 100px; border-radius: 6.4px; object-fit: cover; flex-shrink: 0; margin-right: 20px;" - /> + <img + src="{{item.thumbnail}}" + alt="{{item.title}}" + width="100" + height="100" + style="box-sizing: border-box; width: 100px; height: 100px; border-radius: 6.4px; object-fit: cover; flex-shrink: 0; margin-right: 20px; display: block;" + />추가 팁:
- 일부 클라이언트에서 border-radius 또한 제한될 수 있습니다. 시각 일관성이 매우 중요하다면 모서리 라운딩은 서버쪽에서 처리된 사각형 이미지를 제공하는 방법도 고려해 보세요.
🧹 Nitpick comments (3)
templates/insights/user_weekly_trend.html (3)
18-25
: 신규 게시글 문구 조건부 노출 로직은 의도에 부합. 단, 데이터 안전성 가드 고려 권장new_posts > 0 조건을 문장 내부로 옮겨 가독성과 UX가 개선되었습니다. 다만 템플릿에서 비교 연산을 수행하므로, 컨텍스트에 항상 숫자형(0 이상)이 주입된다는 전제가 필요합니다. 백엔드에서 has_new_posts 같은 불리언을 함께 내려주도록 하면 템플릿 복잡도가 줄고 안전합니다.
- 컨텍스트에서 insight.user_weekly_stats.new_posts가 항상 존재하고 정수형인지 확인해 주세요.
- 원하시면 has_new_posts 플래그 기반으로 리팩터를 제안해 드립니다.
82-84
: 외부 링크 보안 속성 추가는 적절함. 동일 섹션의 다른 분기에도 일관 적용 필요else 분기(현재 라인)에는 target="_blank"와 rel="noopener noreferrer"가 추가됐지만, days_ago 분기(라인 62–69)의 동일 링크에는 누락되어 있습니다. 이메일/웹뷰 모두에서 일관된 보안 특성을 유지하도록 동일 속성을 적용해 주세요.
아래는 days_ago 분기(라인 62–69)의 앵커에 보안 속성을 맞추는 예시입니다(참고용, 실제 라인 범위 밖 변경):
<a href="https://velog-dashboard.kro.kr/leaderboards?based=post&sort=viewCount&limit=10&dateRange=30&utm_source=email&utm_medium=weekly_analysis" style="box-sizing: border-box; color: #999999; font-weight: bold; text-decoration: underline" target="_blank" rel="noopener noreferrer" > 리더보드 </a>
119-121
: 이모지 사용은 가독성 좋음. 일부 환경에서 폰트 미지원 가능성 있음일부 이메일 클라이언트/OS 조합에서 이모지가 기본 폰트로 대체되거나 사라질 수 있습니다. 중요한 정보를 전달하는 요소라면 텍스트 대체 표기를 병행하는 것을 권장합니다(예: "📌" 앞에 "포인트:" 추가).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
templates/insights/index.html
(3 hunks)templates/insights/user_weekly_trend.html
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- templates/insights/index.html
🔇 Additional comments (1)
templates/insights/user_weekly_trend.html (1)
110-113
: 외부 링크 보안 속성 추가는 적절함. URL 안전성(인코딩/정규화) 검증 필요username/slug가 URL 세그먼트로 직접 삽입됩니다. 일반적으로 슬러그는 URL-safe지만, 데이터 품질 보증 전제입니다. 업스트림에서 반드시 정규화/검증되도록 확인해 주세요. 쿼리스트링은 UTM만 포함되어 안전하지만, 경로 세그먼트는 인코딩 누락 시 깨질 수 있습니다.
- username, slug의 URL-safe 보장(정규식 검증 및 저장 시 강제) 여부 확인
- 가능하면 백엔드에서 완성 URL을 생성해 템플릿에는 그대로 주입하는 방식으로 책임을 일원화
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
templates/insights/user_weekly_trend.html (1)
62-69
: 외부 링크 target/rel 속성 적용 불일치 (보안/일관성 이슈)days_ago 분기(라인 62–69)의 리더보드 링크에는 target/rel이 누락되어 있고, else 분기(라인 79–86)에는 적용되어 있습니다. 이메일에서 새 탭 열기 동작의 일관성과 reverse tabnabbing 방지를 위해 두 분기 모두 동일하게 적용해 주세요. 또한 AI 요약에는 두 분기 모두 적용됐다고 되어 있어 실제 코드와 불일치합니다.
아래와 같이 days_ago 분기에 target/rel을 추가하세요:
<a href="https://velog-dashboard.kro.kr/leaderboards?based=post&sort=viewCount&limit=10&dateRange=30&utm_source=email&utm_medium=weekly_analysis" - style="box-sizing: border-box; color: #999999; font-weight: bold; text-decoration: underline" + style="box-sizing: border-box; color: #999999; font-weight: bold; text-decoration: underline" + target="_blank" + rel="noopener noreferrer" > 리더보드 </a>Also applies to: 79-86
♻️ Duplicate comments (2)
templates/insights/user_weekly_trend.html (2)
99-112
: 섹션 레이아웃을 flex → 테이블로 전환: 이메일 호환성 개선Outlook 등 주요 메일 클라이언트 호환성을 고려한 테이블 기반 레이아웃 전환이 적절합니다. 이전 리뷰 지적사항을 충실히 반영했습니다.
Also applies to: 138-140
103-107
: 썸네일: width/height 어트리뷰트 추가 권장 (이메일 클라이언트 호환성)여전히 width/height가 스타일에만 지정되어 있습니다. 일부 클라이언트에서 스타일이 무시되거나 object-fit 미지원으로 레이아웃이 깨질 수 있어, 속성으로도 명시하세요. display:block 추가는 잘 하셨습니다.
- <img - src="{{item.thumbnail}}" - alt="{{item.title}}" - style="box-sizing: border-box; width: 100px; height: 100px; border-radius: 6.4px; object-fit: cover; display: block; margin: 0;" - /> + <img + src="{{item.thumbnail}}" + alt="{{item.title}}" + width="100" + height="100" + style="box-sizing: border-box; width: 100px; height: 100px; border-radius: 6.4px; object-fit: cover; display: block; margin: 0;" + />추가로, 가능하다면 서버 측에서 100x100으로 썸네일을 미리 리사이즈/크롭해 object-fit 의존도를 낮추는 것을 권장합니다.
🧹 Nitpick comments (2)
templates/insights/user_weekly_trend.html (2)
109-110
: 스페이서 셀은 빈 콘텐츠 대신 로 채워 안정성 확보일부 메일 클라이언트에서 빈 가 폭을 유지하지 못하는 사례가 있습니다. 불가시 공백을 넣어 간격 유지 신뢰도를 높이세요.
- <td style="box-sizing: border-box; width: 20px; padding: 0; margin: 0;"></td> + <td style="box-sizing: border-box; width: 20px; padding: 0; margin: 0;"> </td>
124-125
: key_points 노출 길이 제어(선택): 과도한 길이로 인한 줄바꿈 붕괴 방지키 포인트가 많은 경우 모바일/좁은 화면에서 줄바꿈이 과도해질 수 있습니다. 2~3개만 노출하고 말줄임을 권장합니다.
예시:
-{% if item.username and item.key_points %} - <p style="box-sizing: border-box; margin: 0 0 6px 0; font-size: 13px; font-weight: 700; color: #acacac; letter-spacing: 0; line-height: 14.4px;"> - @{{item.username}} | 📌 {{ item.key_points|join:", " }} - </p> +{% if item.username and item.key_points %} + {% set points = item.key_points|slice(0, 3) %} + <p style="box-sizing: border-box; margin: 0 0 6px 0; font-size: 13px; font-weight: 700; color: #acacac; letter-spacing: 0; line-height: 14.4px;"> + @{{item.username}} | 📌 {{ points|join(", ") }}{% if item.key_points|length > 3 %} 외 {{ item.key_points|length - 3 }}개{% endif %} + </p>단, 템플릿 엔진별로 slice 필터 지원 여부가 다를 수 있어 사용 중인 엔진(Jinja/Django 등)에 맞게 조정이 필요합니다.
Also applies to: 131-133
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
templates/insights/user_weekly_trend.html
(4 hunks)
🔇 Additional comments (2)
templates/insights/user_weekly_trend.html (2)
18-25
: 신규 게시글 문구 노출 조건 이동 — 의도에 부합하며 가독성 향상문장 내부로 조건을 옮겨, 신규 글이 없을 때도 나머지 통계 문장이 유지되는 흐름이 좋아졌습니다. 주석도 명확합니다.
114-117
: 제목 링크에 target/rel 추가 — 보안·경험 측면에서 적절외부 링크에 대해 새 탭과 noopener 적용은 안전하고 일관된 사용자 경험을 제공합니다. 잘 반영되었습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다!!!💪
피드백 모두 잘 반영해주신 것 같아 다른 이견은 없습니다!!
Velog Dashboard 글자 위치 깨지는 부분도 잘 반영해주셔서 너무 좋은 것 같아요!👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생 많으셨습니다~~ 다은님이 교차 검증을 너무 잘해주셔서 두 분 다 너무너무 고생많으셨습니다!
@six-standard @ooheunda 🙏🙇🏻♂️
🔥 변경 사항
슬랙에서 제공해주신 뉴스레터 레이아웃 문제들을 해결하였습니다!
다만, 유저 프로필 레이아웃은 당장 정확히 확인할 수 없고, 다음 뉴스레터 발송 후에 알 수 있을 것 같네요;;
자세한 수정 내용과 설명은 해당 태스크를 확인해주시면 됩니다!
🏷 관련 이슈
📸 스크린샷 (UI 변경 시 필수)
before

after

before

after

📌 체크리스트
Summary by CodeRabbit