-
Notifications
You must be signed in to change notification settings - Fork 66
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
[Bug] SettingPicker icon not rendered in combination with ChoicesConfiguration.pickerDisplayMode(.menu) #44
Comments
Code used while debugging in struct SettingPicker_Previews: PreviewProvider {
static var previews: some View {
PreviewWrapper()
}
struct PreviewWrapper: View {
@State private var selectedIndex = 0
var body: some View {
VStack {
SettingPicker(
icon: .system(icon: "chevron.compact.down", backgroundColor: .black),
title: "SettingPicker ~ Navigation",
choices: ["Option 1", "Option 2"],
selectedIndex: $selectedIndex,
choicesConfiguration: SettingPicker
.ChoicesConfiguration(
pickerDisplayMode: .navigation
)
)
SettingPicker(
icon: .system(icon: "chevron.compact.up", backgroundColor: .black),
title: "SettingPicker ~ Menu",
choices: ["Option 1", "Option 2"],
selectedIndex: $selectedIndex,
choicesConfiguration: SettingPicker
.ChoicesConfiguration(
pickerDisplayMode: .menu
)
)
}
}
}
} |
…menu` _ref aheze#44 This small PR fixes bug in aheze#44 where the icon is not showing in SettingPicker component, when used in combination with `Setting.ChoicesConfiguration(pickerDisplayMode: .menu, ....)`: - Preappends `SettingIconView(...)` when using `.menu` picker's display mode - Requires merge of base PR @ aheze#43 & aheze#42 or cherry-picking
Here is the patch diff that fixes this When-original-devs-wont-merge-good-PRs@fac1bd9: index f255d41..fd90f83 100644
--- a/Sources/Views/SettingPicker.swift
+++ b/Sources/Views/SettingPicker.swift
@@ -182,6 +182,10 @@ struct SettingPickerView: View {
case .menu:
HStack(spacing: horizontalSpacing) {
+ if let icon {
+ SettingIconView(icon: icon)
+ }
+
Text(title)
.fixedSize(horizontal: false, vertical: true)
.frame(maxWidth: .infinity, alignment: .leading) Usage: PR not yet created due to merge requirements of #43. Otherwise, cherry-picking and commit by @aheze is also appreciated. The implementation of Patch above fixes |
While experimenting with PR #19 by @Saik0s, I've found a bug which results in
SettingPicker
component not rendering the icon, as it should. This happens when used in combination withChoicesConfiguration.pickerDisplayMode(.menu)
instead of revolving around default.navigation
display mode, which works.Bug Reproduction
To reproduce this bug and make icon not showing use the following code (notably
choiceConfiguration: (...)
constructor param:Example of working code when the icon is shown, using
.navigation
display mode instead:Rendered Results
Left side is working example when using
.navigation
display mode, while the right side is showing bugged version. The important item is the "Vehicle Model" settings (last item).The text was updated successfully, but these errors were encountered: