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

시간대 직접 선택 화면에서, 완료 버튼과 X 버튼이 겹치는 현상 해결 #389

Merged
merged 4 commits into from
Jan 27, 2025

Conversation

plgafhd
Copy link
Collaborator

@plgafhd plgafhd commented Jan 24, 2025

No description provided.

@plgafhd plgafhd requested a review from a team as a code owner January 24, 2025 05:39
@plgafhd
Copy link
Collaborator Author

plgafhd commented Jan 24, 2025

원래 왼쪽 사진처럼 되고 있어서, 시간 선택 중일때만 X 버튼을 없앴어

@JuTaK97
Copy link
Collaborator

JuTaK97 commented Jan 24, 2025

이거 언제부터 이랬지?

@JuTaK97
Copy link
Collaborator

JuTaK97 commented Jan 24, 2025

@JuTaK97
Copy link
Collaborator

JuTaK97 commented Jan 24, 2025

#366 (review)

@plgafhd
Copy link
Collaborator Author

plgafhd commented Jan 24, 2025

Comment on lines 174 to 180
if (optionSheetMode != OptionSheetMode.TimeSelect) {
closeBottomSheetPlaceable.placeRelative(
tagTypePlaceable.width + tagListPlaceable.width - 52.dp.toPx().roundToInt(),
(SearchOptionSheetConstants.TopMargin.toPx().roundToInt() - 32.dp.toPx().roundToInt()) / 2,
)
}
if (baseAnimatedFloat.value != 0f) dragSheetPlaceable.placeRelative(0, 0)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

코드 구조를 이해하니 좀 더 좋은 방법을 찾아냈어
근데, L174의 반대인 optionSheetMode == OptionSheetMode.TimeSelect
L180의 baseAnimatedFloat.value != 0f랑 완벽히 동치인데,
별 이유 없으면 L180도 바꿀까...? 전자가 좀 더 의미 전달이 명확한듯 해서

Copy link
Collaborator

Choose a reason for hiding this comment

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

읽기엔 이게 낫네 굿ㅋㅋ

Copy link
Collaborator

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.

주석 추가
완료!

Copy link
Collaborator

@JuTaK97 JuTaK97 left a comment

Choose a reason for hiding this comment

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

넘좋습니당

@plgafhd plgafhd merged commit 686c09f into develop Jan 27, 2025
3 checks passed
@plgafhd plgafhd deleted the plgafhd/fix-timeselect-cancelbutton branch January 27, 2025 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants