-
Notifications
You must be signed in to change notification settings - Fork 28
QuickEditor height adjustments for the new Scopes #635
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: feature/quick_editor_about_info
Are you sure you want to change the base?
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.
Pull Request Overview
This PR refactors QuickEditor height logic and detent configuration to support new scope options while adding subtle UI improvements. Key changes include updating modal detents resolution in multiple components, introducing a debug log for current detent changes, and adding an animated content size modifier in the About editor.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
gravatar-quickeditor/src/main/java/com/gravatar/quickeditor/ui/editor/extensions/QuickEditorExtensions.kt | Updated modal detents reference to use scopeOption.modalDetents() directly. |
gravatar-quickeditor/src/main/java/com/gravatar/quickeditor/ui/editor/bottomsheet/GravatarQuickEditorBottomSheet.kt | Refactored modal detents logic, added a debug log for current detent, and adjusted visibility for the 'peek' detent. |
gravatar-quickeditor/src/main/java/com/gravatar/quickeditor/ui/editor/QuickEditorViewModel.kt | Removed a private initialPage property in favor of a restructured, shared internal property. |
gravatar-quickeditor/src/main/java/com/gravatar/quickeditor/ui/editor/QuickEditorScopeOption.kt | Added an internal initialPage getter to streamline scope-based page selection. |
gravatar-quickeditor/src/main/java/com/gravatar/quickeditor/ui/abouteditor/AboutEditor.kt | Enhanced the AboutEditor container with an animateContentSize modifier for smoother UI transitions. |
Comments suppressed due to low confidence (1)
gravatar-quickeditor/src/main/java/com/gravatar/quickeditor/ui/editor/bottomsheet/GravatarQuickEditorBottomSheet.kt:332
- [nitpick] Consider renaming the local variable (e.g. to 'computedInitialDetent') to avoid the ambiguity with the function name 'initialDetent'.
val initialDetent = initialDetent(windowHeightSizeClass)
...c/main/java/com/gravatar/quickeditor/ui/editor/bottomsheet/GravatarQuickEditorBottomSheet.kt
Outdated
Show resolved
Hide resolved
dacb127
to
d739e3f
Compare
📲 You can test the changes from this Pull Request in Gravatar Demo by scanning the QR code below to install the corresponding build.
|
d739e3f
to
e7cad99
Compare
e7cad99
to
51d27a9
Compare
Is this what you mean? I see that when opening the Avatar Picker on the Avatar & About scope, with horizontal layout, the initial height of the sheet is smaller than the content and some part of the bottom will get hidden under the screen. It works good on Avatar Picker scope. |
For this reason also (Sticky save button), we decided to remove the save button from landscape mode with keyboard |
I remember like we wanted to always display the Save button. (The landscape + keyboard open mode is an exceptional case we discovered later though). But the idea to me was that the input fields inside the borders would be scrollable not the button. We can get some early design feedback once the feature is in a testable/mature state I think. |
@pinarol - Do you mean to keep it this way on Android for now? |
Description
This PR adjusts various aspect of the QE height in the new scopes:
animateContentHeight
modifier to animated About editor height changes (this applies to any content change but in general when switching from loading to loaded or error state).Note: I've noticed a weird behavior in the AvatarAndAbout scope. With scrolling set to horizontal and the initial page to Avatar picker, the sheet reverts to Peek detent instead of FullyExpanded. I have a feeling this is rather related to the AvatarPicker UI hierarchy so I would like to work on it separately (GRA-103).
Testing Steps