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

Make the whole items in advanced settings screen clickable, standarize paddings #2314

Merged
merged 12 commits into from
Jan 30, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ fun AdvancedSettingsView(
},
trailingContent = ListItemContent.Switch(
checked = state.isRichTextEditorEnabled,
onChange = { state.eventSink(AdvancedSettingsEvents.SetRichTextEditorEnabled(it)) },
),
onClick = { state.eventSink(AdvancedSettingsEvents.SetRichTextEditorEnabled(!state.isRichTextEditorEnabled)) }
)
ListItem(
headlineContent = {
Expand All @@ -78,8 +78,8 @@ fun AdvancedSettingsView(
},
trailingContent = ListItemContent.Switch(
checked = state.isDeveloperModeEnabled,
onChange = { state.eventSink(AdvancedSettingsEvents.SetDeveloperModeEnabled(it)) },
),
onClick = { state.eventSink(AdvancedSettingsEvents.SetDeveloperModeEnabled(!state.isDeveloperModeEnabled)) }
)
ListItem(
headlineContent = {
Expand All @@ -90,8 +90,8 @@ fun AdvancedSettingsView(
},
trailingContent = ListItemContent.Switch(
checked = state.isSendPublicReadReceiptsEnabled,
onChange = { state.eventSink(AdvancedSettingsEvents.SetSendPublicReadReceiptsEnabled(it)) },
),
onClick = { state.eventSink(AdvancedSettingsEvents.SetSendPublicReadReceiptsEnabled(!state.isSendPublicReadReceiptsEnabled)) }
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Row
import androidx.compose.material3.CheckboxColors
import androidx.compose.material3.CheckboxDefaults
import androidx.compose.material3.minimumInteractiveComponentSize
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
Expand Down Expand Up @@ -57,7 +58,7 @@ fun Checkbox(
onCheckedChange(!checked)
}
},
modifier = modifier,
modifier = modifier.minimumInteractiveComponentSize(),
enabled = enabled,
colors = colors,
interactionSource = interactionSource,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Row
import androidx.compose.material3.RadioButtonColors
import androidx.compose.material3.RadioButtonDefaults
import androidx.compose.material3.minimumInteractiveComponentSize
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
Expand All @@ -48,7 +49,7 @@ fun RadioButton(
androidx.compose.material3.RadioButton(
selected = selected,
onClick = onClick,
modifier = modifier,
modifier = modifier.minimumInteractiveComponentSize(),
enabled = enabled,
colors = colors,
interactionSource = interactionSource,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.padding
import androidx.compose.material3.SwitchColors
import androidx.compose.material3.SwitchDefaults
import androidx.compose.material3.minimumInteractiveComponentSize
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
Expand Down Expand Up @@ -52,7 +53,7 @@ fun Switch(
Material3Switch(
checked = checked,
onCheckedChange = onCheckedChange,
modifier = modifier,
modifier = modifier.minimumInteractiveComponentSize(),
enabled = enabled,
colors = colors,
interactionSource = interactionSource,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,11 @@ import androidx.compose.foundation.clickable
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding
import androidx.compose.runtime.Composable
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.semantics.Role
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import io.element.android.libraries.designsystem.components.avatar.AvatarData
import io.element.android.libraries.designsystem.components.avatar.AvatarSize
import io.element.android.libraries.designsystem.preview.ElementThemedPreview
Expand Down Expand Up @@ -58,7 +56,6 @@ fun CheckableUnresolvedUserRow(
)

Checkbox(
modifier = Modifier.padding(end = 16.dp),
checked = checked,
onCheckedChange = null,
enabled = enabled,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,10 @@ package io.element.android.libraries.matrix.ui.components
import androidx.compose.foundation.clickable
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding
import androidx.compose.runtime.Composable
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.semantics.Role
import androidx.compose.ui.unit.dp
import io.element.android.libraries.designsystem.components.avatar.AvatarData
import io.element.android.libraries.designsystem.theme.components.Checkbox

Expand Down Expand Up @@ -54,8 +52,6 @@ fun CheckableUserRow(
)

Checkbox(
modifier = Modifier
.padding(end = 16.dp),
checked = checked,
onCheckedChange = null,
enabled = enabled,
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Copy link
Member

Choose a reason for hiding this comment

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

Interesting. I am trying to see what can cause this.

Copy link
Member

Choose a reason for hiding this comment

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

OK, so I think removing this line will fix this regression. Glad to have screenshot test!

Copy link
Member

Choose a reason for hiding this comment

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

It will match what we have in CheckableUserRow (which is missing a Preview...)

Copy link
Member Author

Choose a reason for hiding this comment

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

It should be fixed in 7c794f5.

Let's see the result in the screenshots.

Copy link
Member

Choose a reason for hiding this comment

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

Now I am wondering if the fix could be to add the padding where it was missing, instead of removing the existing one. The Checkbox are closer to the edge now... Maybe add a 8.dp padding?

Also maybe CheckableUnresolvedUserRow and CheckableUserRow could use a common Composable generic layout.

Copy link
Member Author

Choose a reason for hiding this comment

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

True, I didn't realise that. The 8dp padding will probably solve it, yes 👍 .

Also maybe CheckableUnresolvedUserRow and CheckableUserRow could use a common Composable generic layout.

I can try to make a common one.

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Loading