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

Scrollbars followup, Textareas, Textfields, and more #457

Closed
wants to merge 67 commits into from
Closed

Conversation

hamen
Copy link
Collaborator

@hamen hamen commented Jul 18, 2024

  • TextAread and TextFields are now based on Compose 1.7 implementation
  • Tabstrip scrollbar have a new style

@rock3r
Copy link
Collaborator

rock3r commented Jul 23, 2024

Please add screenshots and videos of the scrollbars in action, and comparable Swing implementations, to help validate they look and behave as expected.

Copy link
Collaborator

@rock3r rock3r left a comment

Choose a reason for hiding this comment

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

Please try to revert unneded code formatting changes, and split out both the standalone sample shortcuts (#462) and BTF2 introduction (#296) to keep this simple and self-contained.

@@ -11,7 +11,6 @@
<inspection_tool class="LanguageDetectionInspection" enabled="false" level="WARNING" enabled_by_default="false" />
<inspection_tool class="NoButtonGroup" enabled="false" level="WARNING" enabled_by_default="false" />
<inspection_tool class="NoScrollPane" enabled="false" level="WARNING" enabled_by_default="false" />
<inspection_tool class="StructuralWrap" enabled="false" level="INFORMATION" enabled_by_default="false" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please revert this. The StructuralWrap inspection is incredibly unhelpful 🙃

Copy link
Collaborator

Choose a reason for hiding this comment

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

Still TODO

Comment on lines 48 to 150
state: TextFieldState,
enabled: Boolean,
readOnly: Boolean,
outline: Outline,
undecorated: Boolean,
keyboardOptions: KeyboardOptions,
singleLine: Boolean,
maxLines: Int,
interactionSource: MutableInteractionSource,
style: InputFieldStyle,
textStyle: TextStyle,
modifier: Modifier = Modifier,
decorationBox: @Composable (innerTextField: @Composable () -> Unit, state: InputFieldState) -> Unit,
showScrollbar: Boolean = true,
) {
var inputState by remember(interactionSource) {
mutableStateOf(InputFieldState.of(enabled = enabled))
}
remember(enabled) { inputState = inputState.copy(enabled = enabled) }

LaunchedEffect(interactionSource) {
interactionSource.interactions.collect { interaction ->
when (interaction) {
is FocusInteraction.Focus -> inputState = inputState.copy(focused = true)
is FocusInteraction.Unfocus -> inputState = inputState.copy(focused = false)
}
}
}

val colors = style.colors
val backgroundColor by colors.backgroundFor(inputState)
val shape = RoundedCornerShape(style.metrics.cornerSize)

val backgroundModifier =
Modifier.thenIf(!undecorated && backgroundColor.isSpecified) {
background(backgroundColor, shape)
}

val borderColor by style.colors.borderFor(inputState)
val hasNoOutline = outline == Outline.None
val borderModifier =
Modifier.thenIf(!undecorated && borderColor.isSpecified && hasNoOutline) {
border(
alignment = Stroke.Alignment.Center,
width = style.metrics.borderWidth,
color = borderColor,
shape = shape,
)
}

val contentColor by colors.contentFor(inputState)
val mergedTextStyle = style.textStyle.merge(textStyle).copy(color = contentColor)
val caretColor by colors.caretFor(inputState)

val lineLimits =
when {
singleLine -> SingleLine
else -> MultiLine(maxLines)
}

val scrollState = rememberScrollState()
val canScroll by remember {
derivedStateOf {
scrollState.canScrollBackward || scrollState.canScrollForward
}
}

Box(
modifier = modifier
.then(backgroundModifier)
.then(borderModifier)
.thenIf(!undecorated && hasNoOutline) { focusOutline(inputState, shape) }
.outline(inputState, outline, shape, Stroke.Alignment.Center),
) {
BasicTextField(
modifier = Modifier
.fillMaxWidth()
.align(Alignment.CenterStart)
.thenIf(canScroll && showScrollbar) { padding(end = 12.dp) },
state = state,
enabled = enabled,
readOnly = readOnly,
textStyle = mergedTextStyle,
cursorBrush = SolidColor(caretColor),
keyboardOptions = keyboardOptions,
lineLimits = lineLimits,
interactionSource = interactionSource,
decorator = { innerTextField: @Composable () -> Unit -> decorationBox(innerTextField, inputState) },
scrollState = scrollState,
)

if (showScrollbar)
VerticalScrollbar(
scrollState = scrollState,
modifier = Modifier.align(Alignment.CenterEnd),
interactionSource = interactionSource,
)
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

To keep this PR more legible, would it be possible to split the text area scrollbars work to a separate PR?

scrollbarVisibility = scrollbarVisibility,
)

public fun ScrollbarStyle.Companion.winOsDark(
Copy link
Collaborator

Choose a reason for hiding this comment

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

windowsAndLinux* here and below?

Copy link
Collaborator Author

@hamen hamen Aug 6, 2024

Choose a reason for hiding this comment

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

Done in a37a196

)
}
var alwaysVisible by remember { mutableStateOf(false) }
val resolvedStyle = readStyle()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this meant to be done to detect the initial style? I think the issue here is that this will be re-executed on every composition. You should probably call this "baseStyle", and remember(JewelTheme.isDark) it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done in 9af3d55

hamen added a commit that referenced this pull request Aug 2, 2024
Signed-off-by: Ivan Morgillo <[email protected]>
hamen added a commit that referenced this pull request Aug 5, 2024
Signed-off-by: Ivan Morgillo <[email protected]>
@hamen hamen mentioned this pull request Aug 5, 2024
@hamen hamen changed the base branch from main to im-fancy-scrollbars August 5, 2024 14:02
@hamen hamen changed the base branch from im-fancy-scrollbars to main August 5, 2024 14:02
@hamen hamen changed the title Add scrollbars Scrollbars followup, Textareas, Textfields, and more Aug 5, 2024
hamen added 13 commits August 5, 2024 17:32
Signed-off-by: Ivan Morgillo <[email protected]>
Signed-off-by: Ivan Morgillo <[email protected]>
Signed-off-by: Ivan Morgillo <[email protected]>
Signed-off-by: Ivan Morgillo <[email protected]>
Signed-off-by: Ivan Morgillo <[email protected]>
Signed-off-by: Ivan Morgillo <[email protected]>
Signed-off-by: Ivan Morgillo <[email protected]>
Signed-off-by: Ivan Morgillo <[email protected]>
Signed-off-by: Ivan Morgillo <[email protected]>
Signed-off-by: Ivan Morgillo <[email protected]>
@hamen hamen marked this pull request as ready for review August 5, 2024 15:43
Signed-off-by: Ivan Morgillo <[email protected]>
if (isHighlighted) {
style.colors.thumbBackgroundHovered
} else {
style.colors.thumbBackgroundHovered
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
style.colors.thumbBackgroundHovered
style.colors.thumbBackground

if (isHighlighted) {
style.colors.thumbBackgroundHovered
} else {
style.colors.thumbBackgroundHovered
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
style.colors.thumbBackgroundHovered
style.colors.thumbBackground

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done in 70d2cda

Comment on lines 50 to 63
state: TextFieldState,
enabled: Boolean,
readOnly: Boolean,
outline: Outline,
undecorated: Boolean,
keyboardOptions: KeyboardOptions,
singleLine: Boolean,
maxLines: Int,
interactionSource: MutableInteractionSource,
style: InputFieldStyle,
textStyle: TextStyle,
modifier: Modifier = Modifier,
decorationBox: @Composable (innerTextField: @Composable () -> Unit, state: InputFieldState) -> Unit,
showScrollbar: Boolean,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
state: TextFieldState,
enabled: Boolean,
readOnly: Boolean,
outline: Outline,
undecorated: Boolean,
keyboardOptions: KeyboardOptions,
singleLine: Boolean,
maxLines: Int,
interactionSource: MutableInteractionSource,
style: InputFieldStyle,
textStyle: TextStyle,
modifier: Modifier = Modifier,
decorationBox: @Composable (innerTextField: @Composable () -> Unit, state: InputFieldState) -> Unit,
showScrollbar: Boolean,
state: TextFieldState,
enabled: Boolean,
readOnly: Boolean,
outline: Outline,
undecorated: Boolean,
keyboardOptions: KeyboardOptions,
singleLine: Boolean,
maxLines: Int,
interactionSource: MutableInteractionSource,
style: InputFieldStyle,
textStyle: TextStyle,
showScrollbar: Boolean,
modifier: Modifier,
decorationBox: @Composable (innerTextField: @Composable () -> Unit, state: InputFieldState) -> Unit,

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done in e494398

.editorconfig Outdated
@@ -16,11 +16,14 @@ indent_size = 2
[*.yml]
indent_size = 2

# noinspection EditorConfigKeyCorrectness
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please revert this file.

Comment on lines +192 to +193
Modifier.defaultMinSize(minWidth = minSize.width, minHeight = minSize.height)
.padding(style.metrics.contentPadding),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please revert this.

Comment on lines +237 to +239
public value class InputFieldState(
public val state: ULong,
) : FocusableComponentState {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please revert this.

Comment on lines +284 to +288
(if (enabled) Enabled else 0UL) or
(if (focused) Focused else 0UL) or
(if (hovered) Hovered else 0UL) or
(if (pressed) Pressed else 0UL) or
(if (active) Active else 0UL),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please revert this.

Comment on lines +310 to +312
derivedStateOf {
isHovered || dragInteraction.value is DragInteraction.Start
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please revert this.

Comment on lines +367 to +368
thumbColor,
RoundedCornerShape(style.metrics.thumbCornerSize),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please revert this.

Comment on lines +238 to +239
measurables
.single { it.layoutId == TEXT_AREA_ID }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please revert this.

hamen added 2 commits August 6, 2024 17:43
@hamen hamen changed the base branch from main to im-108-add-scrollbars-to-textareas August 7, 2024 08:26
@hamen hamen changed the base branch from im-108-add-scrollbars-to-textareas to main August 7, 2024 08:27
@hamen hamen closed this Aug 7, 2024
@hamen hamen deleted the im-419 branch August 26, 2024 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consistency Our UI presentation is not consistent with IJ feature New feature or request
Projects
None yet
2 participants