Feature: Expansion of Preferences Manager #96
Conversation
|
|
We should probably disable this lexicographic ordering, not sure about benefits? |
There was a problem hiding this comment.
Looks good to me 🚀
I like idea of "parent" and "child" in recycler viewer to simplify logic.
I just see potential area where we can improve:
Unify targeted and all preferences logic and presentation.
To me, it seems that both logic and presentations are same(ish). For targeted prefs we just pass "filter". And for all prefs we pass empty "filter"/all included filter (depending on how logic works out)
Could we use just one fragment and one VM for both things? If needed we can add nav argument to make distinction which screen is in question.
Correct me if I am wrong. And if you see potential issues.
| /** | ||
| * Used to initialize Sentinel. | ||
| * | ||
| * @param tools list of tools you want to have e.g. ChuckerTool | ||
| * @param targetedPreferences map of preference files and their properties, used to make finding specific | ||
| * properties more easily, if properties list is empty returns all properties of that file | ||
| * | ||
| * **Example:** | ||
| * // This will contain only tools which are default to Sentinel | ||
| * Sentinel.watch() | ||
| * | ||
| * // This will contain default Sentinel tools + Chucker | ||
| * Sentinel.watch(setOf(ChuckerTool())) | ||
| * | ||
| * // This will contain default Sentinel tools + Chucker and all preference properties of MY_PREFS_FILE | ||
| * Sentinel.watch(setOf(ChuckerTool()), mapOf("MY_PREFS_FILE" to emptyList())) | ||
| * | ||
| * // This will contain default Sentinel tools + Chucker and only MY_PROPERTY property of MY_PREFS_FILE | ||
| * Sentinel.watch(setOf(ChuckerTool()), mapOf("MY_PREFS_FILE" to listOf("MY_PROPERTY"))) | ||
| */ |
There was a problem hiding this comment.
I have questions:
- I know this might counter your design goal (selected prefs and all prefs on separate screens). But, can you watch all preferences in this selected preferences quick view? How? Regardless of answer could we add an example in docs to watch 2 separate files?
- How does the iOS version behave in the default state where a user of lib doesn't specify which preferences they want to watch
There was a problem hiding this comment.
I know this might counter your design goal (selected prefs and all prefs on separate screens). But, can you watch all preferences in this selected preferences quick view? How? Regardless of answer could we add an example in docs to watch 2 separate files?
In selected preferences quick view, you can monitor multiple files but you have to provide names of those files. If that's what you are asking. IMO, it makes sense since app will have dozens of not so relevant files which are then cluttering your UI [but you can access them in AllFiles].
How does the iOS version behave in the default state where a user of lib doesn't specify which preferences they want to watch
To my knowledge, they don't have option to display all files, they just display what clients want. They also have some logic of mapping where clients can provide readable names of properties (instead of KEY_ONE they can send mapping "key one" which is more readable lets say.) I will have to double check with them just in case.
| internal class TargetedPreferencesCollector( | ||
| private val context: Context, | ||
| private val targetedPreferences: Map<String, List<String>> | ||
| ) : Collectors.TargetedPreferences { | ||
|
|
||
| override fun invoke(): List<PreferencesData> { | ||
| if (targetedPreferences.isEmpty()) return emptyList() | ||
|
|
||
| return targetedPreferences.mapNotNull { (fileName, keys) -> | ||
| val allPrefs = context.getSharedPreferences(fileName, MODE_PRIVATE).all | ||
|
|
||
| val tuples = allPrefs.keys | ||
| .filter { keys.isEmpty() || keys.contains(it) } | ||
| .mapNotNull { key -> | ||
| @Suppress("UNCHECKED_CAST") | ||
| when (val value = allPrefs[key]) { | ||
| is Boolean -> Triple(PreferenceType.BOOLEAN, key, value) | ||
| is Float -> Triple(PreferenceType.FLOAT, key, value) | ||
| is Int -> Triple(PreferenceType.INT, key, value) | ||
| is Long -> Triple(PreferenceType.LONG, key, value) | ||
| is String -> Triple(PreferenceType.STRING, key, value) | ||
| is Set<*> -> Triple(PreferenceType.SET, key, value as Set<String>) | ||
| else -> null | ||
| } | ||
| } | ||
|
|
||
| if (tuples.isNotEmpty()) PreferencesData(fileName, tuples) else null |
There was a problem hiding this comment.
Could this be included in regular PreferencesCollector?
As far as I see only difference is using targetedPreferences as source. We could use targetedPreferences as parameter
Correct me if I am wrong.
There was a problem hiding this comment.
I think it can be done. I would just have to add additional filtering of files since there we also fetch all preference files. 👍
| } else { | ||
| emptyList() | ||
| } |
What is the source of this "problem" |
I was being a bit lazy 😄 , but this can be done yes. We will just need some feature flag to toggle visible views since we have that all preferences button, and I plan on adding search functionality limited to |
I think for lexicographic ordering it was detekt. We should probably update rules. |
Yea sure, I will open task for it |
Summary
To make finding specific preferences easier, I decided to split our logic for displaying preferences into 2 parts: all preferences and targeted ones.
Changes
PreferencesTabis now split into 2 parts, tab itself containsTargetedPreferencesand there is a button that leads toAllPreferences. I added additional collector that will collect only files and properties sent by clients viawatchfunction.Recycler
Since a lot data was being collected, Sentinel started stuttering and crashing sometimes. I switched item displaying to recyclerView. Adapter has been a bit harder to write since we had a list of data that also contains a list of data, and everything needs to be displayed. SOLUTION: I went with flattening the preferences data and 2 view holders, where one is preference file item and one is preference property (or child as I called it)
Type
Additional information
Description
TargetedPreferences
SpecificPreferences
Checklist
Additional notes
I will upload documentation a bit later.
Currently if you update an item in
AllPreferencesand go back toTargetedPreferences,that change won't be shown there. I will try to fix it when I find time, but I don't think it is a blocker for now.Also, since we have many files, if you minimize a couple of files and they are still visible on the screen when you click to change a preference property and save it, all other files will expand and you will be scrolled away from that specific preference[if minimized items are not visible, this won't happen]. This is because when preference property is changed, we reload entire screen. We would have to adjust logic so we don't reload entire screen but this can be done later.