Skip to content

Conversation

@TahaTesser
Copy link
Member

@TahaTesser TahaTesser commented Jan 27, 2022

fixes #68249

minimal code sample
import 'package:flutter/material.dart';

void main() => runApp(const MyApp());

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

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Material App',
      theme: ThemeData.dark(),
      home: const TimePickerSample(),
    );
  }
}

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

  Future<TimeOfDay?> _showTimePicker(BuildContext context) {
    return showTimePicker(
      initialTime: TimeOfDay.now(),
      initialEntryMode: TimePickerEntryMode.input,
      context: context,
      confirmText: 'Accept',
      cancelText: 'Reject',
      hourLabelText: 'Happy Hour',
      minuteLabelText: 'Happy Minute',
      helpText: 'Spend your time wisely',
      onEntryModeChanged: (TimePickerEntryMode mode) {
        // ignore: avoid_print
        print('Current Mode: $mode');
      },
    );
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: const Text('TimePicker Sample'),
      ),
      body: Center(
        child: OutlinedButton(
          onPressed: () => _showTimePicker(context),
          child: const Text('Show TimePicker'),
        ),
      ),
    );
  }
}

Native behavior with input mode

The hour text field has the "next" input action which shifts the focus from the hour field to the minute field.
The minute text field has the "done" input action, which hides the keyboard.

native_test.mov

Bug

Both hour and minute text fields have "done" action which when pressing does nothing.

Screen.Recording.2022-05-02.at.00.16.06.mov

Fix

Screen.Recording.2022-05-02.at.00.16.40.mov

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Jan 27, 2022
@TahaTesser TahaTesser added the f: date/time picker Date or time picker widgets label Jan 27, 2022
Copy link
Contributor

@darrenaustin darrenaustin left a comment

Choose a reason for hiding this comment

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

@TahaTesser thanks for the contribution.

This looks reasonable, but I think it could be simplified by not creating and managing the focus nodes. in the _TimePickerInputState. See my comments below. Thx.

@TahaTesser TahaTesser force-pushed the timepicker_inputaction_fix branch from 873fd7a to 532fab1 Compare April 13, 2022 13:10
@TahaTesser TahaTesser force-pushed the timepicker_inputaction_fix branch from 5ec3671 to 0e17174 Compare May 1, 2022 20:02
@TahaTesser TahaTesser requested a review from darrenaustin May 1, 2022 21:18
@TahaTesser
Copy link
Member Author

TahaTesser commented May 1, 2022

@darrenaustin
Removed the focus nodes added earlier, moved the Focus code into conditions, and updated the test and PR description.

Can you please take a look? Thank you!

Copy link
Member

@guidezpl guidezpl left a comment

Choose a reason for hiding this comment

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

  _    ___ _____ __  __ 
 | |  / __|_   _|  \/  |
 | |_| (_ | | | | |\/| |
 |____\___| |_| |_|  |_|
                        

Any objections @darrenaustin ?

@TahaTesser
Copy link
Member Author

Waiting for @darrenaustin's confirmation too before merging this, thanks.

@TahaTesser TahaTesser force-pushed the timepicker_inputaction_fix branch from 0e17174 to 979bbee Compare June 14, 2022 14:01
Copy link
Contributor

@darrenaustin darrenaustin left a comment

Choose a reason for hiding this comment

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

Yikes, so sorry for the lag on this. LGTM. Thx!

@darrenaustin darrenaustin added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 13, 2022
@auto-submit auto-submit bot merged commit f5ebf41 into flutter:master Sep 13, 2022
@TahaTesser TahaTesser deleted the timepicker_inputaction_fix branch September 14, 2022 07:23
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App f: date/time picker Date or time picker widgets f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

showTimePicker not accepting the Done button on the keyboard

3 participants