[Feature] Preferences: New Input Types #27

manuelvrhovac · 2023-09-12T09:26:43Z

Background

We use Sentinel a lot in our project and having a toggle that the QA team (or us devs) can use to test different scenarios in the app has helped us immensely! E.g. Instead of creating several different builds with various configurations, one DEV build with some sentinel toggles was all we needed.

Motivation

However it was limited to only a Boolean input yet I wanted QA and devs to be able to change more parameters like URLs, some time intervals, and just various cases/enums in general during the runtime.

For this reason we've decided it was worth some time to expand the Preferences feature of Sentinel! 🎉

Information

More data types are now available as Preferences: String, Integer, Double, and Enums:

Please let me know what you think! ✨

…uble, and Enums

manuelvrhovac · 2023-09-12T09:38:35Z

Sentinel/Classes/Core/PreferencesInfo/PreferenceItem.swift

+        /// Double input with minimum and maximum values
+        case double(min: Double?, max: Double?)
+        /// Enum input, as a RawRepresentable with either String or Int as type.
+        case enumeration(allCases: [any RawRepresentable])


I thought about having a simple array of strings here as input type. But enums in swift carry more information - both their label and their raw value, e.g if for case north = "N" I can display "north (N)".

Think about the selection possibility - can you have a multiselect or only one selection?

For now only single selection is possible. Here you are supposed to provide all the selectable cases of an enum.

manuelvrhovac · 2023-09-12T09:46:12Z

@nikolamajcen the Bitrise check seems to be failing due to "Xcode test for iOS (Failed)" yet I don't see any tests in this repository? What gives?

goranbrl

Left some comments that should be checked out. Also, we should update the Podspec and the Package.swift files in order to properly release this as a new version.

@nikolamajcen should this be a 1.2 or a 2.0 release, based on the scope of the changes?

goranbrl · 2023-09-12T09:56:35Z

README.md

@@ -66,7 +66,7 @@ The `sourceScreenProvider` object is a type of `SourceScreenProvider` which shou

 The `tools` object is an array of `Tool` objects. `Tool` objects represent tools which will be available from *Sentinel*. There are multiple tools already supported by the library, but custom tools can be created and added to the *Sentinel*.

-The last, but not the least, is the `preferences` object which is an array of `OptionSwitchItem` objects. `OptionSwitchItem` is used to allow the user to switch of some of the preferences which are contained in the app. e.g. The app supports Analitycs and you can add an `OptionSwitchTool` which will be shown on the `Preferences` screen and the user can turn it off if he doesn't want it.
+The last, but not the least, is the `preferences` object which is an array of `PreferenceItem` objects. `PreferenceItem` is used to allow the user to switch of some of the preferences which are contained in the app. e.g. The app supports Analitycs and you can add an `OptionSwitchTool` which will be shown on the `Preferences` screen and the user can turn it off if he doesn't want it.


Suggested change

The last, but not the least, is the `preferences` object which is an array of `PreferenceItem` objects. `PreferenceItem` is used to allow the user to switch of some of the preferences which are contained in the app. e.g. The app supports Analitycs and you can add an `OptionSwitchTool` which will be shown on the `Preferences` screen and the user can turn it off if he doesn't want it.

The last, but not the least, is the `preferences` object which is an array of `PreferenceItem` objects. `PreferenceItem` is used to allow the user to switch off some of the preferences that are contained in the app. e.g. The app supports Analytics and you can add an `OptionSwitchTool` which will be shown on the `Preferences` screen and the user can turn it off if they don't want it.

goranbrl · 2023-09-12T10:06:24Z

Sentinel/Classes/Core/PreferencesInfo/PreferenceItem.swift

@@ -0,0 +1,194 @@
+//
+//  OptionGenericItem.swift


Suggested change

// OptionGenericItem.swift

// PreferenceItem.swift

goranbrl · 2023-09-12T10:17:01Z

Sentinel/Classes/Core/PreferencesInfo/PreferenceItem.swift

+            userDefaults: userDefaults) { newValue in
+                if let setter, let newValue {
+                    setter(newValue.boolValue)
+                }
+            }


Suggested change

userDefaults: userDefaults) { newValue in

if let setter, let newValue {

setter(newValue.boolValue)

}

}

userDefaults: userDefaults

) { newValue in

if let setter, let newValue {

setter(newValue.boolValue)

}

}

goranbrl · 2023-09-12T10:18:05Z

Sentinel/Classes/Core/PreferencesInfo/PreferenceItem.swift

+// MARK: - Deprecated Methods
+
+/// Deprecated. Previously named "OptionSwitchItem" and only supported switch/boolean.
+@available(*, deprecated, renamed: "OptionGenericItem")


Suggested change

@available(*, deprecated, renamed: "OptionGenericItem")

@available(*, deprecated, renamed: "PreferenceItem")

Nice, I like that you went the deprecation route like this 👌

goranbrl · 2023-09-12T10:21:11Z

Sentinel/Classes/Core/PreferencesInfo/PreferenceTableCell.swift

+    @IBOutlet var _titleLabel: UILabel!
+    @IBOutlet var _infoLabel: UILabel!
+    @IBOutlet var _switch: UISwitch!
+    @IBOutlet var _numberField: UITextField!
+    @IBOutlet var _textField: UITextField!
+    @IBOutlet var _enumButton: MenuButton!


All of these can be private, and we should remove the underscores

goranbrl · 2023-09-12T10:25:11Z

MenuButton.swift

+/// A button that opens a context menu with enum cases (raw representables) as options.
+class MenuButton: UIButton {
+
+    private var _enumCases: [any RawRepresentable] = []


Let's ditch the underscore naming for private properties to keep things in line with our code style 🙂

nikolamajcen

Hey, this is a great initiative, and I like it. I've added some comments so please check them out 👍

nikolamajcen · 2023-09-13T07:30:04Z

Example/Sentinel/AppDelegate.swift

+                min: -50,
+                max: 50,
+                userDefaultsKey: "com.infinum.sentinel.optionSwitch.temperature",
+                onDidSet: { [weak self] in self?.didChange(temperature: $0) }


You exposed what happens when the value changes, but preference items need both a getter and a setter.

The reason for the getter is an initial value or getting the value at a specific time. While the setter should be exposed to changing the value outside of this implementation, check how Bool preference values work.

We can discuss if an additional parameter onDidSet is needed because the value will be set automatically with a setter. This applies for all implementations in this file.

For me it made more sense to simply use UserDefaults for both getting and setting the value. An optional onDidSet can be provided to apply the change right away.

If there should be a custom getter for this, then it should go another route and not use userDefaultsKey. I've already considered this and had an enum with either userDefaults or custom as cases of saving/retrieving the value, but it got too complex quickly.

The problem is not user defaults but the suite you use. We should not assume that UserDefaults.standard is used. This can be different, based on use case (sharing between apps, testing suite, etc.).

nikolamajcen · 2023-09-13T07:31:35Z

Example/Sentinel/AppDelegate.swift

+enum Weekday: String, CaseIterable {
+    case monday = "MON"
+    case tuesday = "TUE"
+    case wednesday = "WED"
+    case thursday = "THU"
+    case friday = "FRI"
+    case saturday = "SAT"
+    case sunday = "SUN"
+}
+
+enum Direction: Int, CaseIterable {
+    case north = 1
+    case west = 2
+    case east = 3
+    case south = 4
+}


I would recommend moving this to a separate file to make the app delegate clean as possible.

nikolamajcen · 2023-09-13T07:36:25Z

Sentinel/Classes/Core/PreferencesInfo/PreferenceItem+Init.swift

+    /// - Parameter userDefaultsKey: The key under which this preference is saved in UserDefaults
+    /// - Parameter userDefaults: Optionally define a different suite of UserDefaults (standard by default).
+    /// - Parameter onDidSet: Optional execution block once the value is changed.
+    public convenience init(


Based on your changes, I suggest making this more generic.
I recommend injecting a type (enum) instead of having multiple inits (for text, bool, etc.).

Also, I would go with a Validator protocol and inject different validators for different types (it can be an array of validators if you like). E.g., what happens now if the text should be all uppercase and the length should be max 5 characters? It is hard to maintain current implementation by adding additional parameters to the init method.

It would be good to add getter and setter in the init (like bool).

In short, you'll have basically one init instead of multiple ones.

This was the original idea. The problem was however they type of the value in the onDidSet closure - it was String? and not Double or Int. Having these convenience initializers enabled me to define the exact type that is handled.

Otherwise it would have to be a generic initializer, and I would have to check whether T is supported (String, Int, Double...)

convenience init<T>(name: String, inputType: InputType, type: T.Type, onDidSet: (T?) -> Void)

nikolamajcen · 2023-09-13T07:38:43Z

Sentinel/Classes/Core/PreferencesInfo/PreferenceItem.swift

+        /// Double input with minimum and maximum values
+        case double(min: Double?, max: Double?)
+        /// Enum input, as a RawRepresentable with either String or Int as type.
+        case enumeration(allCases: [any RawRepresentable])


Think about the selection possibility - can you have a multiselect or only one selection?

nikolamajcen · 2023-09-13T07:39:18Z

Sentinel/Classes/Core/PreferencesInfo/PreferenceItem.swift

+            }
+            value = String(number)
+        case .double(let minimum, let maximum):
+            newValue = newValue.replacingOccurrences(of: ",", with: ".") // Swift only recognizes dot as decimal separator.


Are you sure about that? Can you test this with different regional settings?

Yup, the Double initialiser only accepts dot as the decimal separator, irregardless of region.
So many regions use comma so I added this as a tiny improvement, not planning on implementing number formatter here really.

Sentinel/Classes/Core/PreferencesInfo/PreferenceItem.swift

nikolamajcen · 2023-09-13T07:45:03Z

@goranbrl with the current changes, as I can see, we can still use the same code for boolean values. So, with that in mind, I would go with 1.2.0.

But, based on the changes that I've proposed, that could change a bit, and changes would be required for value initialization (not backward compatible). If that happens, the change should require a new major version, and that could be 2.0.0 - based on semantic versioning.

nikolamajcen · 2023-09-13T07:54:37Z

Hey @manuelvrhovac, there are test targets, but that was not the issue. In short, I've fixed the CI, and it works as expected now 😉

More data types are now available as Preferences: String, Integer, Do…

13c349d

…uble, and Enums

manuelvrhovac added the enhancement New feature or request label Sep 12, 2023

manuelvrhovac assigned nikolamajcen and goranbrl Sep 12, 2023

manuelvrhovac commented Sep 12, 2023

View reviewed changes

goranbrl requested changes Sep 12, 2023

View reviewed changes

nikolamajcen assigned manuelvrhovac and unassigned nikolamajcen and goranbrl Sep 12, 2023

nikolamajcen self-requested a review September 12, 2023 11:03

nikolamajcen requested changes Sep 13, 2023

View reviewed changes

goranbrl requested a review from ZvonimirMedak September 14, 2023 07:22

PR fixes

0ce48d2

nikolamajcen mentioned this pull request Feb 21, 2024

[WIP] Support for different preference input types #32

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] Preferences: New Input Types #27

[Feature] Preferences: New Input Types #27

manuelvrhovac commented Sep 12, 2023 •

edited

Loading

manuelvrhovac Sep 12, 2023

nikolamajcen Sep 13, 2023

manuelvrhovac Sep 14, 2023

manuelvrhovac commented Sep 12, 2023

goranbrl left a comment

goranbrl Sep 12, 2023

goranbrl Sep 12, 2023

goranbrl Sep 12, 2023

goranbrl Sep 12, 2023

goranbrl Sep 12, 2023

goranbrl Sep 12, 2023

goranbrl Sep 12, 2023

nikolamajcen left a comment

nikolamajcen Sep 13, 2023

manuelvrhovac Sep 14, 2023

nikolamajcen Sep 14, 2023 •

edited

Loading

nikolamajcen Sep 13, 2023

nikolamajcen Sep 13, 2023

nikolamajcen Sep 13, 2023

manuelvrhovac Sep 14, 2023

nikolamajcen Sep 13, 2023

nikolamajcen Sep 13, 2023

manuelvrhovac Sep 14, 2023

nikolamajcen commented Sep 13, 2023

nikolamajcen commented Sep 13, 2023

	The last, but not the least, is the `preferences` object which is an array of `PreferenceItem` objects. `PreferenceItem` is used to allow the user to switch of some of the preferences which are contained in the app. e.g. The app supports Analitycs and you can add an `OptionSwitchTool` which will be shown on the `Preferences` screen and the user can turn it off if he doesn't want it.
	The last, but not the least, is the `preferences` object which is an array of `PreferenceItem` objects. `PreferenceItem` is used to allow the user to switch off some of the preferences that are contained in the app. e.g. The app supports Analytics and you can add an `OptionSwitchTool` which will be shown on the `Preferences` screen and the user can turn it off if they don't want it.

	@available(*, deprecated, renamed: "OptionGenericItem")
	@available(*, deprecated, renamed: "PreferenceItem")

[Feature] Preferences: New Input Types #27

Are you sure you want to change the base?

[Feature] Preferences: New Input Types #27

Conversation

manuelvrhovac commented Sep 12, 2023 • edited Loading

Background

Motivation

Information

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manuelvrhovac commented Sep 12, 2023

goranbrl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikolamajcen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikolamajcen Sep 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikolamajcen commented Sep 13, 2023

nikolamajcen commented Sep 13, 2023

manuelvrhovac commented Sep 12, 2023 •

edited

Loading

nikolamajcen Sep 14, 2023 •

edited

Loading