-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fix/style/layout responsiveness/set up #93
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary
Nice work on this! I tested it on my own device (pup-up window to resize) and it works seamlessly on most window sizes, with some exceptions.
I noticed you used LazyColumn
s instead of normal Column
s, more on this in code comment.
Important
Code Quality
The code is generally unchanged, with modifications made to the padding values and Column
changes to LazyColumn
. Out of the scope of this issue but we'll need to clean it up a bit.
I believe padding management should be relegated to the Theme.kt
, where you put default values for XS to XL-size padding and apply them everywhere. This 9min video explains it pretty well, but sadly it does not go Jetpack Compose into implementation detail.
This would make it easier to standardize across screens as well, so I think any work you'd put into it now is time gained later on.
Functionality
Because the emulator doesn't work on my device, I ran the app directly on my phone and used the pop-up and split-screen view to try and resize it. It works very well on all full-width sized windows (where it touches both left and right borders), but when resizing it with small width, the layout is still shaky:
At ~3/4 of normal width:
At ~1/2 of normal witdh:
For reference, my device screen size is a Samsung A32 (size: 6.4 inches, resolution: 1080 x 2400 pixels, ratio: 20:9).
Testing
All UI tests still pass
Documentation
Not applicable (not the scope of this issue but will need reworking)
Suggestions
I believe either setting a minimum width for the app or having dynamically resized text would solve the layout issue with small widths, but I have not done any further research on the subject.
Steps before PR approved
- Fix layout with small widths
- Standardize padding using
Theme.kt
@@ -79,124 +80,132 @@ fun SignInScreen(authViewModel: AuthViewModel, navigationActions: NavigationActi | |||
// Purple-ish background | |||
GradedBackground(Purple80, Pink40, PurpleGrey80, "signInBackground") | |||
|
|||
Column( | |||
modifier = Modifier.fillMaxSize().padding(padding).padding(60.dp), | |||
LazyColumn( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use LazyColumn
s and not just use Modifier.verticalScroll(rememberScrollState())
instead?
From what I understand, LazyColumn
s are most useful when there are a lot of elements on the page and we don't want to load all of them if they're not displayed.
@charliemangano |
@francelu there is already a default font defined in |
Set Up Responsive Dimensions and Typography for All Screens
Description
This PR establishes a foundation for responsive design by setting up scalable dimensions and typography that adapt to different screen sizes. This setup allows screens to adjust automatically, ensuring consistency across devices. Subtask from #82.
Changes
Files
Added
Dependencies Added
androidx.compose.material3:material3-window-size-class:1.3.0
for window size class handling to support responsive layouts.Testing
Screenshots (if applicable)
Include examples of the scalable elements in action if needed.