-
Notifications
You must be signed in to change notification settings - Fork 0
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
Display content edge-to-edge #91
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.
Nicely done, works in most places. I found a few screens that you probably have missed:
- When clicking on a lecture, the Lecture Units items are cut off at the bottom of the screen
- CreateChat: Clicking the FAB and "Create Chat", the info (eg "Please enter at least 3 characters ...") is hidden behind the keyboard
Also, have you tried to adding an UI test that asserts that the ReplyTextField is still visibile after the Keyboard popped up? I am not sure whether compose tests simulate the keyboard, though.
I think we should definetly somehow ensure that the TextField is visible with the Keyboard popping up, as this is like the main usecase for our app.
...um/informatics/www1/artemis/native_app/feature/metis/conversation/ui/thread/MetisThreadUi.kt
Outdated
Show resolved
Hide resolved
Thanks, for pointing that out, I'll go ahead and make the required changes.
That sounds like a really good idea. I'll try to investigate this further and see if I can implement another test class, that adds textfield visibility tests. |
...ww1/artemis/native_app/feature/metis/conversation/ui/reply/ReplyTextFieldVisibilityUITest.kt
Outdated
Show resolved
Hide resolved
...ww1/artemis/native_app/feature/metis/conversation/ui/reply/ReplyTextFieldVisibilityUITest.kt
Outdated
Show resolved
Hide resolved
...ww1/artemis/native_app/feature/metis/conversation/ui/reply/ReplyTextFieldVisibilityUITest.kt
Outdated
Show resolved
Hide resolved
...atics/www1/artemis/native_app/feature/metis/conversation/ConversationAnswerMessagesUITest.kt
Outdated
Show resolved
Hide resolved
...ww1/artemis/native_app/feature/metis/conversation/ui/reply/ReplyTextFieldVisibilityUITest.kt
Show resolved
Hide resolved
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.
Lgtm now
Description
This PR introduces edge-to-edge content, which will be enforced with Android 15 (SDK 35).
The changes were made by following the official documentation.
For lazy lists, the content now goes all the way to the edge of the screen leading to the new intended way to display content. Therefore the bottom padding has been replaced with content padding. imePadding has been introduced for items, that normally stick to the end of the screen, but should still be visible, when the ime changes its size.
This PR closes #68.
Testing
The changes have been tested on foldables, tablets and both screen orientations.
Screenshots
Content consumes system bar:
Content padding to keep content in the safe area: