-
-
Notifications
You must be signed in to change notification settings - Fork 19
fix wide buttons on larger screens #196
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
base: master
Are you sure you want to change the base?
fix wide buttons on larger screens #196
Conversation
Thank you! I think in the long run an entirely different layout for tablets would be needed, but this is definitely an improvement over those extremely wide buttons 👍🏼 |
Not sure whether we need to be as strict as 320dp, I think that looks a bit weird on the large screens. Perhaps 400dp or even 500dp might be fine? |
Up to you. I just went with 320 because anything over that got Android Studio fussing at me with a soft "warning" that >320dp is too wide. 🤷🏻♂️ Again, the 320 number might be updated or removed entirely once the Material Design folks figure their documentation out. |
I also updated the touch target size to 48dp so things are easier to hit with a finger (most noticeable on the color selector) |
Maybe create a value |
The widest view show in my visual summaries is a desktop screen, which I highly doubt anyone will be using anything that large, so you can probably disregard the largest samples above lol |
Having a single place to define the value would be amazing and should be way easier to maintain. |
I found maxWidth limitation to be unnecessary in the dialog layouts, as they're already pretty nicely contained to the popup. Removed those constraints from the XML for simplicity. activity_report_crash is a little goofy with being all left-aligned right now. |
Yeah, I can't get the activity_report_crash layout to center cleanly, so I'm leaving it alone. |
app:layout_constraintTop_toBottomOf="@id/pin_shortcut_appbar"> | ||
|
||
<LinearLayout | ||
<androidx.constraintlayout.widget.ConstraintLayout |
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.
I think changing the LinearLayout to a ConstraintLayout makes the layout rather hard to read and hard to maintain.
Every child of the ConstraintLayout needs to "know" the one before, i.e. when adding or removing something those references need to be updated. Furthermore the constraintWidth_max is repeated again and again.
What about wrapping the LinearLayout in a ConstraintLayout like this?
<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="wrap_content">
<LinearLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:gravity="center"
android:orientation="vertical"
android:padding="10dp"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent"
app:layout_constraintWidth_max="500dp">
...
That way the contents of the LinearLayout don't need to be modified.
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.
Alternatively, one could restrict the width of the LinearLayout like this and use android:layout_gravity="center_horizontal".
I think that is the cleanest solution, since it doesn't require nesting the layouts.
dc3bebb
to
2cda589
Compare
for #83
In general, this limits buttons to 320dp, per Material Design "best practice"(?), but this "best practice" may be changing according to this issue thread here.
Also increases touch target size to a more accessible 48dp.
Without things crashing, I couldn't find a clean way to get the app drawer list off the left edge of the screen (on larger screens). This likely has to do with how the ImageView and TextViews for the apps are expected somewhere upstream when the drawer is launched, but I don't have the time or brainpower to trace that one out yet.