element-hq / element-hq/element-x-android
Classes that extend PreviewParameterProvider<T> confusingly suffixed with just "Provider"
- Dominant language
- Kotlin
- Stars
- 2.4k
- Forks
- 624
- Avg merge
- 1d 14h
- Merged PRs (30d)
- 149
Description
Proposal: suffix them with PreviewParam instead.
"Preview" alone is not a good candidate as is already used as a suffix for functions as part of a well established pattern.
"Params" alone is not a good candidate as is already used as a data class and is maybe a bit too general.
"PreviewParams" as a string is only present in one .java file for the Camera so little chance for confusion there.
"PreviewParam" with no -s doesn't appear to be used anywhere.
Why: make dev onboarding less painful, speed up debugging
Status Quo:
If I understand correctly, Previews get their "sample" data from PreviewParameterProviders which are configured in a separate file/class per View and called from that View. At first glance, the naming conventions is (PackageName)View / (PackageName)Provider. These classes for creating "sample" data for Previews are not production code. However, Matrix and Element-X both contain numerous features that are called "Provider" as well as the concept being a strong pattern in production code all over the project. Sometimes a "Provider" suffix means "not production code, just preview stuff" and sometimes it is production code. So you have to double think when searching for usages. Additionally, we get brain cancer inducing names when a provider feature also has a View and thus has a Provider for the Preview for the View of the Provider.
Examples:
AccountProviderProvider is just the preview param for the View for AccountProvider
changeaccountprovider is a production feature. It has a View,
but there is no ChangeAccountProviderProvider, but rather
ChangeAccountProviderStateProvider for the preview param for ChangeAccountProviderView
not to be confused with: changeAccountProviderNodeProvider (which is part of a factory)
Could instead be:
AccountProviderPreivewParam for the View for AccountProvider
changeaccountprovider has has a ChangeAccountProviderView
for which there is, naturally, a ChangeAccountProviderPreviewParam. I don't need to parse what type each View operates on while searching for usages.
Now PreviewParam classes are immediately understandable and can be safely and easily ignored by new and old devs alike unless they are actually working on previews.
(PackageName)+State+Provider doesn't exist to provide a state, it exists to provide a FAKE state to use as a PreviewParameter. PreviewParamererProvider abstracts out which Class is needed for the View to run (for good reason), we don't need to preserve it in a class naming convention when the Class and the ViewPreview function both clearly say it. We don't have multiple PreviewParameterProvider per package to de-conflict. Trying really hard to think of a reason why I would want to quickly know from a class name if it is a PreviewParameterProvider of a simple object or of a State, and I can't. And the status quo is so confusing, I'm not sure if that is really currently possible. I don't know why there is a test in KonsistClassNameTest that tries to preserve the parent type name in the class name, so I did a test.
Test performed:
Refactor Rename on
AccountProviderProvider -> AccountProviderPreviewParam
ChangeAccountProviderStateProvider -> ChangeAccountProviderPreviewParam
ConfirmAccountProviderStateProvider -> ConfirmAccountProviderPreivewParam
Gradle sync
No errors reported
Reload all 3 views
No errors noticed
Build App and run on Pixel 3A
No errors
If you switch back and forth a lot, Android Studio is sometimes unhappy after gradle sync and reports errors, but if you just build all the error flags go away and the previews work. There does not appear to be anything about androidx.compose.ui.tooling.preview.PreviewParameterProvider that requires this confusing naming convention where the name of the class extending this interface is dependent on whatever random thing the View runs on instead of the package name where the view lives, or that it needs to end with "Provider".
Contributor guide
Assessment
This issue has not been assessed yet.