Skip to content

Conversation

samtstern
Copy link
Contributor

See #1434

I also updated the sample app in two ways:

  • Combine the ToS and PP selection radio box now that we make you pass them together anyway
  • Allow a 'None' option

Here's how it looks now:
fui_landscape_notos
fui_landscape_withtos

Change-Id: I3afab29958c713b2d53c9c5619244ee1867cfdc4
Change-Id: Ifeb4de6ac932db9e4aaae19269f4a05def2c62e4
style="@style/FirebaseUI.Text.BodyText"
android:layout_width="0dp"
android:layout_height="0dp"
android:layout_height="wrap_content"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to do that too, but then this happens. Maybe you know more ConstraintLayout jitsu? 😆

screenshot_20180925-103634

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm. I probably need to mess with the vertical chain type then. All the "padding" on the ToS/PP text isn't actually padding, it's a result of the view bounds stretching to meet its constraints.

Change-Id: I51011d5e347d4cf569dd03e05d5e0d8ad649be5f
@samtstern
Copy link
Contributor Author

@SUPERCILEX I think at some point with split screen there's no good layout. Here's what I have now at half-screen in both portrait and landscape. IMO it looks as good as we can hope for.

Plus while it's a bummer visually, I think if you have a terms/privacy policy you need them to be visible at all costs.

Half of Lanscape
fui_half_land

Half of Portrait
fui_half_portrait

@SUPERCILEX
Copy link
Collaborator

A few thoughts:

  • Is the large layout-land fixed? Since you reverted wrap_content and didn't add the contraint weights, I'm pretty sure this doesn't fix the original issue 😂
  • The ToS actually scrolls so I don't think it's too big a deal if not all of it is visible at once
  • Small layouts are hard! 🤪

@samtstern
Copy link
Contributor Author

samtstern commented Sep 26, 2018

What do you mean by "large layout-land"? I think that's what I showed in the very first comment, but maybe there's something else?

Also how does the ToS scroll?

@SUPERCILEX
Copy link
Collaborator

The first comment used wrap_content, right? But in the last commit, you changed it back to 0dp so the logo doesn't disappear on small screens... which means we're back to square 1.

When you're in vertical split screen mode, you can move the slider up to squish our poor app even further into oblivion. Then the ToS will be cut off but scrollable.

@samtstern
Copy link
Contributor Author

Yep the landscape (the original bug) is still working with the latest commits:

android_emulator_-_pixel_2_api_28_5554_and_firebaseui-android____projects_firebaseui-android__-_____buildsrc_src_main_kotlin_config_kt__buildsrc_

@SUPERCILEX
Copy link
Collaborator

Ohhh, nevermind you made the ScrollView 0dp. So that means the screenshot below is still a bug on your branch. Were you saying that's ok? That's why I was pointing out that the ToS is scrollable.

I've created a patch with my fix to clear confusion, but it does mean you have to scroll the ToS. WDYT? Is that a reasonable tradeoff?

screenshot_20180926-194021

Change-Id: I47beeb3a5dc47bf84de769adaf7dff830e3807a6
@samtstern
Copy link
Contributor Author

@SUPERCILEX thanks for the patch! I applied it and it definitely helps in the super-small situation.

From what I can tell, this now looks passable in any configuration of options/orientation/size.

Copy link
Collaborator

@SUPERCILEX SUPERCILEX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

@samtstern samtstern merged commit d1a92a2 into version-4.2.1-dev Sep 27, 2018
@samtstern samtstern added this to the 4.2.1 milestone Sep 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants