[Refactor] border의 none, !important 빼기 #124
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
seoyoonyi
requested review from
Panda-raccoon,
clara-shin,
jizerozz and
ssuminii
리뷰어 가이드 by Sourcery이 PR은 두 가지 주요 리팩토링 변경 사항을 구현합니다: 더 나은 브라우저 호환성을 위해 CSS 리팩토링 변경 사항에 대한 클래스 다이어그램classDiagram
class Input {
+border: 0
+border-color: theme.colors.main.alert
}
class Pagination {
+className: active
+background-color: theme.colors.teal[50]
+color: theme.colors.teal[600]
}
class VerificationInput {
+border-color: theme.colors.gray[300]
}
class StrategyTitleSection {
+border: 0
}
class StatisticsTable {
+border: 0
}
class Button {
+border: 0
}
class Terms {
+list-style-type: disc
+margin-left: 16px
}
파일 수준 변경 사항
연결된 문제에 대한 평가
잠재적으로 연결된 문제
팁 및 명령어Sourcery와 상호작용하기
경험 맞춤화대시보드에 액세스하여:
도움 받기Original review guide in EnglishReviewer's Guide by SourceryThis PR implements two main refactoring changes: replacing Class diagram for CSS refactoring changesclassDiagram
class Input {
+border: 0
+border-color: theme.colors.main.alert
}
class Pagination {
+className: active
+background-color: theme.colors.teal[50]
+color: theme.colors.teal[600]
}
class VerificationInput {
+border-color: theme.colors.gray[300]
}
class StrategyTitleSection {
+border: 0
}
class StatisticsTable {
+border: 0
}
class Button {
+border: 0
}
class Terms {
+list-style-type: disc
+margin-left: 16px
}
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
🚀 Deployed on https://673a035a23d885f65ba0bd08--sysmetic.netlify.app |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
안녕하세요 @seoyoonyi - 변경 사항을 검토한 결과 해결해야 할 몇 가지 문제가 발견되었습니다.
차단 문제:
- 프로덕션 컴포넌트에서 테스트 페이지 제거 (링크)
검토 중에 확인한 내용
- 🟢 일반 문제: 모두 양호
- 🔴 보안: 차단 문제 1개
- 🟢 테스트: 모두 양호
- 🟢 복잡성: 모두 양호
- 🟢 문서화: 모두 양호
더 유용하게 도와주세요! 각 댓글에 👍 또는 👎를 클릭해 주시면 피드백을 사용하여 리뷰를 개선하겠습니다.
Original comment in English
Hey @seoyoonyi - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Remove test pages from production component (link)
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🔴 Security: 1 blocking issue
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| <div> | ||
| <h1>나의 전략 리스트</h1> | ||
| <p>트레이더가 마이페이지로 이동했을 때 나타나는 페이지입니다.</p> | ||
| <InputTestPage /> |
There was a problem hiding this comment.
🚨 issue (security): 프로덕션 컴포넌트에서 테스트 페이지 제거
테스트 페이지는 테스트 유틸리티가 최종 사용자에게 노출될 수 있으므로 프로덕션 컴포넌트에 포함되어서는 안 됩니다.
Original comment in English
🚨 issue (security): Remove test pages from production component
Test pages should not be included in production components as they may expose testing utilities to end users.
ssuminii
approved these changes
Nov 17, 2024
Contributor
ssuminii
left a comment
ssuminii
left a comment
There was a problem hiding this comment.
border: none; 처리해도 border-width가 기본값(medium)으로 남아 있을 수 있는걸 첨알았네요.. 감사합니다 승인완료 !! 💗
jizerozz
approved these changes
Nov 17, 2024
Contributor
jizerozz
left a comment
jizerozz
left a comment
There was a problem hiding this comment.
히익 .. 급하게 한다고 확인을 제대로 못했네요 ㅠㅠㅠㅠ 수정해주셔서 감사합니다...
Contributor
|
감사합니다ㅜㅜ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🚀 풀 리퀘스트 제안
closes #120
📋 작업 내용
기본 스타일링으로 돌아가는 문제를 방지하기 위해 border: none; 대신 border: 0;로 변경했습니다.
border: none;은 border-width가 기본값(medium)으로 남아 있는 문제가 발생할 수 있으므로 이를 방지하기 위해 border: 0;을 사용하여 border-width: 0으로 명시적으로 설정했습니다.
!important 사용을 제거했고, 이를 대체하기 위해 두 가지 방법을 적용했습니다.
마이페이지 들어가시면 테스트용 코드 볼수 있습니다.
🔧 변경 사항
주요 변경 사항을 요약해 주세요.
📸 스크린샷 (선택 사항)
📄 기타
추가적으로 전달하고 싶은 내용이나 특별한 요구 사항이 있으면 작성해 주세요.
Sourcery에 의한 요약
기본 스타일링 문제를 방지하기 위해 'border: none;'을 'border: 0;'으로 교체하여 border-width가 명시적으로 0으로 설정되도록 CSS 스타일을 리팩토링합니다. 스타일에서 '!important'를 제거하고 클래스 우선순위와 선택자 재정의를 조정하여 의도한 스타일링을 유지합니다. 테스트 목적으로 TraderMyPage에 테스트 컴포넌트를 추가합니다.
향상된 기능:
테스트:
Original summary in English
Summary by Sourcery
Refactor CSS styles by replacing 'border: none;' with 'border: 0;' to ensure border-width is explicitly set to 0, preventing default styling issues. Remove '!important' from styles and adjust class priorities and selector overrides to maintain intended styling. Add test components to TraderMyPage for testing purposes.
Enhancements:
Tests: