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(BreakChanges): add Modifier arg to Internal**ColumnScrollbar #20

Merged
merged 1 commit into from
Jul 15, 2023
Merged

feat(BreakChanges): add Modifier arg to Internal**ColumnScrollbar #20

merged 1 commit into from
Jul 15, 2023

Conversation

8cAyqpVKio
Copy link
Contributor

Passing Modifier is required.

@nanihadesuka nanihadesuka merged commit e091728 into nanihadesuka:master Jul 15, 2023
1 check passed
@nanihadesuka
Copy link
Owner

Didn't add modifier as the scope of what it should modify for the scrollbar is ambiguous (pass the modifier to the the scroll box area, scrollbar or the whole parent box?) But yours is good enough.

@nanihadesuka nanihadesuka self-requested a review July 15, 2023 09:57
@nanihadesuka nanihadesuka added the enhancement New feature or request label Jul 15, 2023
@8cAyqpVKio
Copy link
Contributor Author

pass the modifier to the the scroll box area, scrollbar or the whole parent box?
No.

For example,

BoxWithConstraints(
modifier = modifier
.alpha(alpha)
.fillMaxWidth()
) {
if (indicatorContent != null) BoxWithConstraints(
Modifier
.align(if (rightSide) Alignment.TopEnd else Alignment.TopStart)
.fillMaxHeight()
.graphicsLayer(
translationX = with(LocalDensity.current) { (if (rightSide) displacement.dp else -displacement.dp).toPx() },
translationY = constraints.maxHeight.toFloat() * normalizedOffsetPosition
)
) {

this modifier

if (indicatorContent != null) BoxWithConstraints(
Modifier
.align(if (rightSide) Alignment.TopEnd else Alignment.TopStart)

is initialized by Modifier, so parent modifier
BoxWithConstraints(
modifier = modifier
.alpha(alpha)
.fillMaxWidth()
) {

don't pass.

@8cAyqpVKio
Copy link
Contributor Author

Thanks for merging the PR.
I think it's okay, but if you have any problems, feel free to revert code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants