Skip to content
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

Merged
merged 29 commits into from
Feb 12, 2024

Conversation

wba2hi
Copy link
Contributor

@wba2hi wba2hi commented Feb 1, 2024

Interesting lectures and documentation:

What did I use:

  • WindowSizeClass to differentiate between Layout and Portrait mode
  • FlowRow and FlowColumn Views for "better flow" on changing screens

What did I not use:

  • A Canonical Layout. Feed does not fit, List-Detail also does not fit. Supporting Pane could fit, however the RamsesView can only "shrink" in size, it can not "increase" in size 100*100 -> 100*50 works but 100*50 -> 100*100 (due to expanding shrinking does not work). Therefore we need something to overlay our view and not to reduce it's size. An element which can expand and shrink again. So I added a custom BottomSheet
    -> 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

@Chrylo
Copy link
Contributor

Chrylo commented Feb 5, 2024

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
Copy link
Contributor

Chrylo commented Feb 5, 2024

Functional:

  • 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.
  • The current tab is not saved when rotating
  • 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?

@wba2hi
Copy link
Contributor Author

wba2hi commented Feb 8, 2024

  • 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.

- 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
@Chrylo
Copy link
Contributor

Chrylo commented Feb 12, 2024

LGTM

  • Tested all views in both landscape + portrait.
  • Tested connection to the Databroker while rotating the views
  • Tested new Sidesheet element
  • Tested state restoration while rotating the views

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

known issues:

@Chrylo Chrylo merged commit e852135 into eclipse-kuksa:main Feb 12, 2024
6 checks passed
@Chrylo Chrylo deleted the feature-20 branch February 12, 2024 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optimize Views for Landscape
2 participants