eclipse-kuksa / kuksa-android-companion

Apache License 2.0
3 stars 1 forks source link

feature: Optimize Views for Landscape #48

Closed wba2hi closed 6 months ago

wba2hi commented 7 months ago

Interesting lectures and documentation:

What did I use:

What did I not use:

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

Chrylo commented 7 months ago

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?

Screenshot 2024-02-05 at 13 27 44

https://m2.material.io/components/sheets-bottom#behavior

Another idea would be to use an expanding Action Button like this: https://stackoverflow.com/questions/75933627/how-to-expand-fab-into-a-sheet-in-jetpack-compose

Chrylo commented 7 months ago

Functional:

wba2hi commented 7 months ago
  • I would not expect that a reconnection (unsubscribe / disconnect etc.) happens during a rotation because only the views should be rebuild. Probably not easy to fix if something is a bit too entangled here. Lets discuss this.

Fixed

  • The current tab is not saved when rotating

Fixed

  • The Tab bar has now more height since the settings button was added there. Is it possible to reduce that again? Is it possible to add some kind of line divider between the settings and the previous button? To show some visual decoupling?

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.

Chrylo commented 6 months ago

LGTM

This was a big task so some improvements will be made in follow up tickets like #55.

known issues: