-
Notifications
You must be signed in to change notification settings - Fork 1
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
feature: Optimize Views for Landscape #48
Conversation
Closes: eclipse-kuksa#20 Signed-off-by: Andre Weber <andre.weber3@etas.com>
Regarding the custom BottomSheet. I really liked how the bottom sheet felt. Can we maybe revert to that one and use a Sidesheet like in the link? https://m2.material.io/components/sheets-bottom#behavior Another idea would be to use an expanding Action Button like this: |
Functional:
|
app/src/main/kotlin/org/eclipse/kuksa/companion/extension/ConfigurationExtension.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/org/eclipse/kuksa/companion/extension/ConstraintScopeExtension.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/org/eclipse/kuksa/companion/feature/connection/view/AnimatedLoadingText.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/org/eclipse/kuksa/companion/feature/connection/view/AnimatedLoadingText.kt
Outdated
Show resolved
Hide resolved
...n/kotlin/org/eclipse/kuksa/companion/feature/connection/view/VerticalConnectionStatusView.kt
Show resolved
Hide resolved
app/src/main/kotlin/org/eclipse/kuksa/companion/feature/connection/view/VerticalText.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/org/eclipse/kuksa/companion/feature/connection/view/VerticalText.kt
Outdated
Show resolved
Hide resolved
...n/kotlin/org/eclipse/kuksa/companion/feature/connection/view/AdaptiveConnectionStatusView.kt
Show resolved
Hide resolved
app/src/main/kotlin/org/eclipse/kuksa/companion/feature/home/view/sheet/AdaptiveLine.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/org/eclipse/kuksa/companion/feature/home/view/sheet/AdaptiveSheetLayout.kt
Outdated
Show resolved
Hide resolved
KTLints property-naming rule is not compatible with compose. The official recommendation is to disable the rule, see: pinterest/ktlint#2139 This should be okay, since we still have the detekt linter, which also checks for names in a more compose compatible way.
Fixed
Fixed
Previously we used a TabHost, now we use a NavigationBar which has a well-defined height. Therefore the new height is intended. Increased the Icon size a bit to compensate for the height of the NavigationBar. |
app/src/main/kotlin/org/eclipse/kuksa/companion/extension/StringExtension.kt
Outdated
Show resolved
Hide resolved
.../main/kotlin/org/eclipse/kuksa/companion/feature/home/view/navigation/NavigationViewModel.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/org/eclipse/kuksa/companion/feature/home/view/sheet/SideSheetView.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/org/eclipse/kuksa/companion/feature/home/view/sheet/SideSheetView.kt
Outdated
Show resolved
Hide resolved
- add feature folder for Navigation and (Side-)/(Bottom-)Sheet
* main: feature: Add Icon for F-Droid to Fastlane chore: Add Info about KUKSA CLI and Python Library chore: Drop ".val" suffix docs: Adapt Documentation as per Review Findings docs: Add Quickstart Guide feature(Build): Remove F-Droid BuildType
LGTM
This was a big task so some improvements will be made in follow up tickets like #55. known issues:
|
Interesting lectures and documentation:
What did I use:
What did I not use:
-> I added AdaptiveViews, which based on the windowSizeClasses show different content (e.g. horizontal orientation instead of vertical), e.g. an AdaptiveNavigation consists of a HorizontalNavigation and a VerticalNavigation which both take there data from the same dataset.
Why AdaptiveViews? Why 2 Views and not if-else?
I tried to keep everything in one view, see AdaptiveSheet. However when switching from Portrait to Landscape a lot of parameter change. When setting the width in Portrait, we need to set the Height in Landscape for example. However we can not easily use kotlins .apply {} or .also {} scope when working with the modifier. It adds a lot of complexity (if portrait use width, else use height) to each view if it tries to do both. I tried to find the best way by adding two separate views, which are "simpler" and "more maintainable" over one view which tries to do both but is error-prone hard to read and less maintainable.
I tried to move all "AdaptiveView"-logic inside the AdaptiveScreen
I moved the settings from being a FloatingActionButton inside the RamsesView to an element inside the NavigationBar
Closes: #20