fix: widen snackbars on tablets (#4921) - #4985
ibrahim-iqbal wants to merge 1 commit into
Conversation
Snackbars used the Material default width on tablets, so they only occupied a narrow band in the middle of the screen and left the rest of the row empty. Add a small extension, applyResponsiveWidth, that expands the snackbar view to match the parent width when the device reports a smallest-width of at least 600dp, matching the sw600dp qualifier the layouts already use. Apply it in the four snackbar helpers in ActivityExt and FragmentExt, and in the inline Snackbar.make sites in FileDisplayActivity, PreviewImageFragment, PreviewAudioFragment, PreviewTextFragment, and PreviewVideoActivity so every snackbar in the app benefits. Phones and other devices below the 600dp threshold keep the current Material width. Signed-off-by: ibrahim-iqbal <ibrahim-iqbal@users.noreply.github.com>
|
|
|
Hi @ibrahim-iqbal! Thanks for opening this PR! 🙌🏻 I'll start with the CR as soon as possible. Stay tuned! |
joragua
left a comment
There was a problem hiding this comment.
Hi @ibrahim-iqbal! 💯 Nice job! The first CR round is here:
- We use to have one commit for calens entry (
chore: add calens file) in each PR. Could you split the commit of this PR in two different ones? Thanks in advance!
Let us know if you have any doubts and we will be happy to help you! 😄
| Bugfix: Widen snackbars on tablets | ||
|
|
||
| Snackbars used the Material default width on tablets, so they only occupied a | ||
| narrow band in the middle of the screen and left the rest of the row empty. | ||
| The width now expands to fill the parent when the device reports a | ||
| smallest-width of at least 600dp, so tablet users see a snackbar that spans | ||
| the row like it does on phones. | ||
|
|
||
| https://github.com/owncloud/android/issues/4921 |
There was a problem hiding this comment.
Please, read the TEMPLATE.md file and apply the guidelines here
- Description must be written in present perfect passive tense
- The name of the file must be the PR ID, in this case
4985 - The link of the PR must be added at the end of the file, just below the issue link
NOTE: Do not create a new commit with all the changes. Apply the suggestions to the same commit that creates the calens file (chore: add calens file)
| /** | ||
| * Expand the snackbar view to fill the width of its parent when the device | ||
| * reports a smallest-width of 600dp or more, so tablets don't render a short | ||
| * snackbar that leaves most of the row empty. Below 600dp the Material default | ||
| * is kept, which centres the snackbar and caps its width for readability. | ||
| */ |
There was a problem hiding this comment.
Is this comment necessary here? 🤔 Comments are not needed when code is self-explanatory, and in my opinion, the applyResponsiveWidth method is descriptive enough
| * is kept, which centres the snackbar and caps its width for readability. | ||
| */ | ||
| fun Snackbar.applyResponsiveWidth(): Snackbar { | ||
| val smallestWidthDp = view.context.resources.configuration.smallestScreenWidthDp |
There was a problem hiding this comment.
Is this variable needed? It is only used for comparison.
You can remove that line and write the method like this:
if (view.context.resources.configuration.smallestScreenWidthDp >= 600) { ... }
Do you agree? 🤔 It's just my opinion...
Related Issues
App: closes #4921
Snackbars used the Material default width on tablets, so they only occupied a narrow band in the middle of the screen and left most of the row empty. This ports the phone-like behaviour to tablets: at a smallest-width of at least 600dp — the same qualifier the existing
layout-sw600dpfolder targets — the snackbar view expands to fill the parent, so a device the size of the Galaxy Tab A8 in the original report gets a snackbar that spans the row.Changes
New helper
owncloudApp/src/main/java/com/owncloud/android/extensions/SnackbarExt.kt:Snackbar.applyResponsiveWidth()readsresources.configuration.smallestScreenWidthDpand, on tablet-sized devices (>= 600dp), sets the snackbar view's layout width toMATCH_PARENT. Below the threshold it returns the snackbar unchanged, so phones keep the current Material width.ActivityExt.ktandFragmentExt.kt: the four helpers (showMessageInSnackbar,showSnackbarWithActionon bothActivityandFragment) chainapplyResponsiveWidth()beforeshow().Inline
Snackbar.make(...)sites that bypass the helpers:FileDisplayActivity.kt:1411,PreviewImageFragment.kt:251/257,PreviewAudioFragment.kt:308/314,PreviewTextFragment.kt:173/181,PreviewVideoActivity.kt:414/422. All now chain the same helper so no snackbar in the app is narrow on a tablet.Changelog file at
changelog/unreleased/4921(Bugfix:type).Release Notes in
ReleaseNotesViewModel.kt— not added, this is a UI polish fix rather than a headline feature; happy to add one if you'd prefer.QA
./gradlew :owncloudApp:compileOriginalDebugKotlin→ BUILD SUCCESSFUL../gradlew :owncloudApp:ktlintCheck— no new violations in the changed files; pre-existing ktlint violations elsewhere onmasterare unchanged.applyResponsiveWidth()returns early and the snackbar keeps its Material default layout params.PreviewImageFragment) on a sw600dp+ tablet (Galaxy Tab A8 in the original report) and confirm the snackbar now spans the row. A phone (< 600dp) should look identical to today.