-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Update Input Screen landscape behavior #6898
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
Update Input Screen landscape behavior #6898
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.
Works as expected 👍
A minor change I would make is to use a slide transition instead of fade transition when going from bottom omnibar to top input screen bar, I think it feels better. This would require adding isLandscape
to BrowserAndInputScreenTransitionProvider
(or passing some activity context) and I'd understand if you don't want to do it here. We can revisit whenever we touch this code next time.
override fun useTopBar(isLandscape: Boolean): Boolean = | ||
useTopBar( | ||
isTopOmnibar = isTopOmnibar, | ||
isTopOmnibar = isTopOmnibar || isLandscape, | ||
duckChatInternal = duckChatInternal, | ||
) |
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.
override fun useTopBar(isLandscape: Boolean): Boolean = | |
useTopBar( | |
isTopOmnibar = isTopOmnibar, | |
isTopOmnibar = isTopOmnibar || isLandscape, | |
duckChatInternal = duckChatInternal, | |
) | |
override fun useTopBar(isLandscape: Boolean): Boolean = | |
useTopBar( | |
isTopOmnibar = isTopOmnibar, | |
duckChatInternal = duckChatInternal, | |
) || isLandscape |
This is a little clearer in intent to me.
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.
Ah sorry, now noticing that other invocations of useTopBar
are not providing the orientation. This results in missing blur and autocomplete recycler offset (which in this context debatable because buttons are to the side), but also the autocomplete arrows point in the wrong direction.
To cover all the cases, you can leverage the new activity context injection into @Inject
@ActivityContext
lateinit var activityContext: Context and do: override fun useTopBar(): Boolean =
useTopBar(
isTopOmnibar = isTopOmnibar,
duckChatInternal = duckChatInternal,
) || activityContext.resources.configuration.orientation == Configuration.ORIENTATION_LANDSCAPE |
Task/Issue URL: https://app.asana.com/1/137249556945/project/488551667048375/task/1211507598155413?focus=true
Description
Steps to test this PR
UI changes