firebase / FirebaseUI-Flutter

Apache License 2.0
92 stars 80 forks source link

🐛 PhoneInputScreen footerBuilder builds before "back" button #145

Closed mvdf95 closed 7 months ago

mvdf95 commented 8 months ago

Is there an existing issue for this?

What plugin is this bug for?

Firebase UI Auth

What platform(s) does this bug affect?

Android, iOS, Web

List of dependencies used.

flutter pub deps -s list
dependencies:
- flutter 0.0.0
  - characters 1.3.0
  - collection 1.17.2
  - material_color_utilities 0.5.0
  - meta 1.9.1
  - vector_math 2.1.4
  - web 0.1.4-beta
  - sky_engine any
- flutter_localizations 0.0.0
  - flutter any
  - intl 0.18.1
  - characters 1.3.0
  - clock 1.1.1
  - collection 1.17.2
  - material_color_utilities 0.5.0
  - meta 1.9.1
  - path 1.8.3
  - vector_math 2.1.4
  - web 0.1.4-beta
- firebase_core 2.17.0
  - firebase_core_platform_interface ^4.8.0
  - firebase_core_web ^2.8.0
  - flutter any
  - meta ^1.8.0
- firebase_auth 4.10.1
  - firebase_auth_platform_interface ^6.19.1
  - firebase_auth_web ^5.8.2
  - firebase_core ^2.17.0
  - firebase_core_platform_interface ^4.8.0
  - flutter any
  - meta ^1.8.0
- firebase_ui_auth 1.9.0
  - email_validator ^2.1.17
  - firebase_auth ^4.10.1
  - firebase_core ^2.17.0
  - firebase_dynamic_links ^5.3.4
  - firebase_ui_localizations ^1.7.0
  - firebase_ui_oauth ^1.4.12
  - firebase_ui_shared ^1.4.0
  - flutter any
  - flutter_localizations any
- firebase_ui_oauth_google 1.2.12
  - firebase_auth ^4.10.1
  - firebase_ui_oauth ^1.4.12
  - flutter any
  - google_sign_in ^6.1.0
- firebase_ui_localizations 1.7.0
  - flutter any
  - flutter_localizations any
  - path ^1.8.2
- provider 6.0.5
  - collection ^1.15.0
  - flutter any
  - nested ^1.0.0
- cupertino_icons 1.0.6

dev dependencies:
- flutter_test 0.0.0
  - flutter any
  - test_api 0.6.0
  - matcher 0.12.16
  - path 1.8.3
  - fake_async 1.3.1
  - clock 1.1.1
  - stack_trace 1.11.0
  - vector_math 2.1.4
  - async 2.11.0
  - boolean_selector 2.1.1
  - characters 1.3.0
  - collection 1.17.2
  - material_color_utilities 0.5.0
  - meta 1.9.1
  - source_span 1.10.0
  - stream_channel 2.1.1
  - string_scanner 1.2.0
  - term_glyph 1.2.1
  - web 0.1.4-beta
- flutter_lints 2.0.3
  - lints ^2.0.0

transitive dependencies:
- _fe_analyzer_shared 61.0.0
  - meta ^1.0.2
- _flutterfire_internals 1.3.7
  - collection ^1.0.0
  - firebase_core ^2.17.0
  - firebase_core_platform_interface ^4.8.0
  - flutter any
  - meta ^1.8.0
- analyzer 5.13.0
  - _fe_analyzer_shared ^61.0.0
  - collection ^1.17.0
  - convert ^3.0.0
  - crypto ^3.0.0
  - glob ^2.0.0
  - meta ^1.7.0
  - package_config ^2.0.0
  - path ^1.8.0
  - pub_semver ^2.0.0
  - source_span ^1.8.0
  - watcher ^1.0.0
  - yaml ^3.0.0
- args 2.4.2
- async 2.11.0
  - collection ^1.15.0
  - meta ^1.1.7
- boolean_selector 2.1.1
  - source_span ^1.8.0
  - string_scanner ^1.1.0
- characters 1.3.0
- clock 1.1.1
- collection 1.17.2
- convert 3.1.1
  - typed_data ^1.3.0
- crypto 3.0.3
  - typed_data ^1.3.0
- desktop_webview_auth 0.0.13
  - crypto ^3.0.3
  - flutter any
  - http ^1.0.0
  - flutter_web_plugins any
  - plugin_platform_interface ^2.1.4
- email_validator 2.1.17
- fake_async 1.3.1
  - clock ^1.1.0
  - collection ^1.15.0
- file 7.0.0
  - meta ^1.9.1
  - path ^1.8.3
- firebase_auth_platform_interface 6.19.1
  - _flutterfire_internals ^1.3.7
  - collection ^1.16.0
  - firebase_core ^2.17.0
  - flutter any
  - meta ^1.8.0
  - plugin_platform_interface ^2.1.3
- firebase_auth_web 5.8.2
  - firebase_auth_platform_interface ^6.19.1
  - firebase_core ^2.17.0
  - firebase_core_web ^2.8.0
  - flutter any
  - flutter_web_plugins any
  - http_parser ^4.0.0
  - js ^0.6.3
  - meta ^1.8.0
- firebase_core_platform_interface 4.8.0
  - collection ^1.0.0
  - flutter any
  - flutter_test any
  - meta ^1.8.0
  - plugin_platform_interface ^2.1.3
- firebase_core_web 2.8.0
  - firebase_core_platform_interface ^4.8.0
  - flutter any
  - flutter_web_plugins any
  - js ^0.6.3
  - meta ^1.8.0
- firebase_dynamic_links 5.3.7
  - firebase_core ^2.17.0
  - firebase_core_platform_interface ^4.8.0
  - firebase_dynamic_links_platform_interface ^0.2.6+7
  - flutter any
  - meta ^1.8.0
  - plugin_platform_interface ^2.1.3
- firebase_dynamic_links_platform_interface 0.2.6+7
  - _flutterfire_internals ^1.3.7
  - firebase_core ^2.17.0
  - flutter any
  - meta ^1.8.0
  - plugin_platform_interface ^2.1.3
- firebase_ui_oauth 1.4.12
  - desktop_webview_auth ^0.0.13
  - firebase_auth ^4.10.1
  - firebase_ui_auth ^1.9.0
  - firebase_ui_shared ^1.4.0
  - flutter_svg ^2.0.7
  - flutter any
- firebase_ui_shared 1.4.0
  - flutter any
- flutter_svg 2.0.7
  - flutter any
  - vector_graphics ^1.1.7
  - vector_graphics_codec ^1.1.7
  - vector_graphics_compiler ^1.1.7
- flutter_web_plugins 0.0.0
  - flutter any
  - characters 1.3.0
  - collection 1.17.2
  - material_color_utilities 0.5.0
  - meta 1.9.1
  - vector_math 2.1.4
  - web 0.1.4-beta
- glob 2.1.2
  - async ^2.5.0
  - collection ^1.15.0
  - file >=6.1.3 <8.0.0
  - path ^1.8.0
  - string_scanner ^1.1.0
- google_identity_services_web 0.2.1+1
  - js ^0.6.4
  - meta ^1.3.0
- google_sign_in 6.1.5
  - flutter any
  - google_sign_in_android ^6.1.0
  - google_sign_in_ios ^5.5.0
  - google_sign_in_platform_interface ^2.4.0
  - google_sign_in_web ^0.12.0
- google_sign_in_android 6.1.20
  - flutter any
  - google_sign_in_platform_interface ^2.2.0
- google_sign_in_ios 5.6.4
  - flutter any
  - google_sign_in_platform_interface ^2.2.0
  - pigeon ^11.0.1
- google_sign_in_platform_interface 2.4.2
  - flutter any
  - plugin_platform_interface ^2.1.0
  - quiver ^3.0.0
- google_sign_in_web 0.12.0+5
  - flutter any
  - flutter_web_plugins any
  - google_identity_services_web ^0.2.1
  - google_sign_in_platform_interface ^2.4.0
  - http >=0.13.0 <2.0.0
  - js ^0.6.3
- http 1.1.0
  - async ^2.5.0
  - http_parser ^4.0.0
  - meta ^1.3.0
- http_parser 4.0.2
  - collection ^1.15.0
  - source_span ^1.8.0
  - string_scanner ^1.1.0
  - typed_data ^1.3.0
- intl 0.18.1
  - clock ^1.1.0
  - meta ^1.0.2
  - path ^1.8.0
- js 0.6.7
  - meta ^1.7.0
- lints 2.1.1
- matcher 0.12.16
  - async ^2.10.0
  - meta ^1.8.0
  - stack_trace ^1.10.0
  - term_glyph ^1.2.0
  - test_api >=0.5.0 <0.7.0
- material_color_utilities 0.5.0
  - collection ^1.15.0
- meta 1.9.1
- nested 1.0.0
  - flutter any
- package_config 2.1.0
  - path ^1.8.0
- path 1.8.3
- path_parsing 1.0.1
  - vector_math ^2.1.0
  - meta ^1.3.0
- petitparser 5.4.0
  - meta ^1.9.0
- pigeon 11.0.1
  - analyzer ^5.13.0
  - args ^2.1.0
  - collection ^1.15.0
  - meta ^1.7.0
  - path ^1.8.0
  - yaml ^3.1.1
- plugin_platform_interface 2.1.6
  - meta ^1.3.0
- pub_semver 2.1.4
  - collection ^1.15.0
  - meta ^1.3.0
- quiver 3.2.1
  - matcher ^0.12.10
- sky_engine 0.0.99
- source_span 1.10.0
  - collection ^1.15.0
  - path ^1.8.0
  - term_glyph ^1.2.0
- stack_trace 1.11.0
  - path ^1.8.0
- stream_channel 2.1.1
  - async ^2.5.0
- string_scanner 1.2.0
  - source_span ^1.8.0
- term_glyph 1.2.1
- test_api 0.6.0
  - async ^2.5.0
  - boolean_selector ^2.1.0
  - collection ^1.15.0
  - meta ^1.3.0
  - source_span ^1.8.0
  - stack_trace ^1.10.0
  - stream_channel ^2.1.0
  - string_scanner ^1.1.0
  - term_glyph ^1.2.0
- typed_data 1.3.2
  - collection ^1.15.0
- vector_graphics 1.1.7
  - flutter any
  - vector_graphics_codec 1.1.7
- vector_graphics_codec 1.1.7
- vector_graphics_compiler 1.1.7
  - args ^2.3.0
  - meta ^1.7.0
  - path_parsing ^1.0.1
  - xml ^6.3.0
  - vector_graphics_codec 1.1.7
- vector_math 2.1.4
- watcher 1.1.0
  - async ^2.5.0
  - path ^1.8.0
- web 0.1.4-beta
- xml 6.3.0
  - collection ^1.17.0
  - meta ^1.9.0
  - petitparser ^5.4.0
- yaml 3.1.2
  - collection ^1.15.0
  - source_span ^1.8.0
  - string_scanner ^1.1.0

Steps to reproduce

PhoneInputScreen(footerBuilder: (context) => Text( 'footer builds between main button and back button'))

image

Expected Behavior

Footer should build after all the buttons, like it does for SignInScreen(), for example.

Actual Behavior

Footer builds between main button and back button.

Additional Information

Immediately apparent on phone_input_screen.dart lines 127 and 133 that the UniversalButton widget is written after PhoneInputView widget builds the footer:

PhoneInputView(
                  auth: auth,
                  action: action,
                  subtitleBuilder: subtitleBuilder,
                  footerBuilder: footerBuilder,
                  flowKey: flowKey,
                  multiFactorSession: multiFactorSession,
                  mfaHint: mfaHint,
                ),
                const SizedBox(height: 8),
                UniversalButton(
                  text: l.goBackButtonLabel,
                  variant: ButtonVariant.text,
                  onPressed: () {
                    Navigator.of(context).pop();
                  },
                ),

image

darshankawar commented 8 months ago

@mvdf95 Can you provide complete runnable reproducible code sample that shows the reported behavior ?

mvdf95 commented 8 months ago

@mvdf95 Can you provide complete runnable reproducible code sample that shows the reported behavior ?

Right. You're going to need to replace your own DefaultFirebaseOptions here because mine contains my Firebase credentials and I'm not sure how to mock them, but other than that is this what you're looking for?


import 'package:flutter/material.dart';
import 'package:firebase_core/firebase_core.dart';
import 'package:firebase_ui_auth/firebase_ui_auth.dart';
import 'firebase/firebase_options.dart' show DefaultFirebaseOptions;

void main() async {
  WidgetsFlutterBinding.ensureInitialized();
  await Firebase.initializeApp(options: DefaultFirebaseOptions.currentPlatform);
  FirebaseUIAuth.configureProviders([PhoneAuthProvider()]);
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Test App',
      theme: ThemeData(primarySwatch: Colors.blue),
      home: PhoneInputScreen(
          footerBuilder: (context) =>
              Text('footer builds between main button and back button')),
    );
  }
}

This is the screen this code generates:

image

darshankawar commented 8 months ago

Thanks for the update. Yes, I do see the same behavior, but I am not sure what exactly is the issue here. Can you elaborate what is the expected behavior as compared to what we are seeing currently ?

mvdf95 commented 8 months ago

@darshankawar

footerBuilder is a property passed on to firebase_ui_auth's screen widgets intended for building a footer (bottom section of the page), yes? A use case would be some copyright text, for example. For SignInScreen and ForgotPasswordScreen, it works as intended as no widgets are built after it:

SignInScreen


import 'package:flutter/material.dart';
import 'package:firebase_core/firebase_core.dart';
import 'package:firebase_ui_auth/firebase_ui_auth.dart';
import 'firebase/firebase_options.dart' show DefaultFirebaseOptions;

void main() async {
  WidgetsFlutterBinding.ensureInitialized();
  await Firebase.initializeApp(options: DefaultFirebaseOptions.currentPlatform);
  FirebaseUIAuth.configureProviders([EmailAuthProvider(), PhoneAuthProvider()]);
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Test App',
      theme: ThemeData(primarySwatch: Colors.blue),
      home: SignInScreen(
          footerBuilder: (context, action) =>
              Text('Copyright MyApp © 2023. All rights reserved.')),
    );
  }
}

image

ForgotPasswordScreen

import 'package:flutter/material.dart';
import 'package:firebase_core/firebase_core.dart';
import 'package:firebase_ui_auth/firebase_ui_auth.dart';
import 'firebase/firebase_options.dart' show DefaultFirebaseOptions;

void main() async {
  WidgetsFlutterBinding.ensureInitialized();
  await Firebase.initializeApp(options: DefaultFirebaseOptions.currentPlatform);
  FirebaseUIAuth.configureProviders([EmailAuthProvider(), PhoneAuthProvider()]);
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Test App',
      theme: ThemeData(primarySwatch: Colors.blue),
      home: ForgotPasswordScreen(
          footerBuilder: (context) =>
              Text('Copyright MyApp © 2023. All rights reserved.')),
    );
  }
}

image

But then PhoneInputScreen renders the footer before the back button, cutting the UI in half. This is inconsistent with the other screens and also makes no sense for the interface, especially if I were to pad the footer a little or add some more text, increasing the distance to the "back" button:

PhoneInputScreen

import 'package:flutter/material.dart';
import 'package:firebase_core/firebase_core.dart';
import 'package:firebase_ui_auth/firebase_ui_auth.dart';
import 'firebase/firebase_options.dart' show DefaultFirebaseOptions;

void main() async {
  WidgetsFlutterBinding.ensureInitialized();
  await Firebase.initializeApp(options: DefaultFirebaseOptions.currentPlatform);
  FirebaseUIAuth.configureProviders([EmailAuthProvider(), PhoneAuthProvider()]);
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Test App',
      theme: ThemeData(primarySwatch: Colors.blue),
      home: PhoneInputScreen(
          footerBuilder: (context) => Padding(
                padding: const EdgeInsets.all(8.0),
                child: Text(
                    'Copyright MyApp © 2023. All rights reserved. Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.'),
              )),
    );
  }
}

image

Does this make the issue clear?

darshankawar commented 8 months ago

Thanks for the update.