Giving the keyboard focus to a Dropdown menu on mobile doesn't seem like the common case, so maybe we should make this a nullable property and compute a default value (if needed) based on the target platform. This would be an incompatible change however DropdownMenu hasn't been out for long.

Fixed! On mobile platforms, the text field will be focusable only when the showVirtualKeyboard is true now. Thanks so much for the suggestion. Please let me know if there're any problems:)

Instead of showVirtualKeyboard maybe this should be takesFocus, since one might set it to false on desktop platforms to prevent searching or filtering?

@gspencergoog - do we have a conventional name for a flag that means "request the focus when clicked or tapped"?

No, we don't have a set name for that, since text fields are the only thing that do that.

takesFocus doesn't include the part about needing user interaction to take focus, it sounds like a synonym for canRequestFocus which is what we currently use for the ability to take focus.

Maybe requestFocusOnTap instead?

But the docs here say "If set to true ... the text field is focusable", implying that if it is set to false, then the text field is not focusable, which I don't think is the case, correct? It's still focusable, it just doesn't request focus on tap, meaning that you could tab to get to it, and it would focus, you just can't tap on it to focus it, am I right?

Also, it seems odd to have it be ignored on some platforms and not others. It seems like desktop platforms could just default to false, and mobile default to true.

requestFocusOnTap sounds better and agreed that the default value would be false for mobile, true otherwise.

No, we don't have a set name for that, since text fields are the only thing that do that.

takesFocus doesn't include the part about needing user interaction to take focus, it sounds like a synonym for canRequestFocus which is what we currently use for the ability to take focus.

Maybe requestFocusOnTap instead?

But the docs here say "If set to true ... the text field is focusable", implying that if it is set to false, then the text field is not focusable, which I don't think is the case, correct? It's still focusable, it just doesn't request focus on tap, meaning that you could tab to get to it, and it would focus, you just can't tap on it to focus it, am I right?

Also, it seems odd to have it be ignored on some platforms and not others. It seems like desktop platforms could just default to false, and mobile default to true.

requestFocusOnTap sounds good to me! Thanks for the suggestion. And yes, I think "focusable" here is not accurate. I need to change the doc.

And will set the default value to false for mobile platforms and true for desktop platforms. Thanks!

It probably doesn't bear mentioning, but: the widget will only create a (private) FocusNode if this parameter is null and requestFocusOnTap is true.

This is tricky. If the focusNode was provided (via widget.focusNode) then we should not dispose it. If requestFocusOnTap is false, then we probably don't need to create a focus node. If the dropdown gets rebuilt with a new value of focusNode or requestFocusOnTap we'll need to update things in didUpdateWidget. For example, the widget might be built with widget.focusNode:null, and then rebuilt with a non-null focus node. A bunch of tests to make sure we do the right thing will be needed.

Will update didUpdateWidget and tests. Thanks a lot!

But for this condition,

If requestFocusOnTap is false, then we probably don't need to create a focus node.

Currently, if the requestFocusOnTap is false, I use the focusNode to unfocus (_textFocusNode.unfocus();) the text field when it is tapped. Is this the correct way to keep the TextField unfocused? It looks like we don't need to create a focus node when requestFocusOnTap is true because the text field will automatically request focus when it's tapped.

You probably shouldn't unfocus the text field like that, since there are edge cases where you'll have it flashing as focused for a frame, or having to unfocus at times when you didn't expect to (e.g. when popping a route).

The long term answer is probably to use a Text widget instead of a TextField when it is not focusable. That is much easier said than done, however, since the Text widget doesn't have an InputDecorator, and we probably want the decoration, and there are a lot of things that the text field does for you here.

In the meantime, though, would setting the readOnly attribute on the text field be sufficient? Probably not: seems like that's kind of what you want, but I think that it ignores taps then, which might not be ideal.

Another alternative would be to always supply a focus node to the text field, and set canRequestFocus on the node to be false. Ugh. No, that won't work, because it is overwritten by the text field, and it only turns it off if the field is disabled.

Okay, how about adding an optional canRequestFocus to the text field, and having that override the text field's internal logic if it is set. Then you can pass "false" for that attribute to the text field, and it won't be allowed to accept focus.

This sounds good to me! Will make the change. Thanks so much:)

Not sure if I've got this all right. Can the developer still give the text field the focus by having its focus node request the focus? ... it looks like the answer is "no"; we should clear up that case here too.

You are right. The text field cannot be focused if false. Sorry for missing this part.

Why did this change?

I changed the padding style for the trailing icon button from EdgeInsets.symmetric(horizontal: 4.0) to EdgeInsets.all(4.0) for some vertical space, so the height of the text field gets changed.

isMobile should really be a late final that's initialized with a switch. per https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#avoid-using-if-chains-or--or--with-enum-values

Fixed. Thanks!

What 'done' button?

This should read something like: the desktop platform closed the menu when .

Sorry about the confusion. It should be the done action when editing is finished. Just updated this comment.

The requestFocusOnTap parameter doesn't appear to be needed here. Hopefully we have a test that proves that one can override the default value of requestFocus for mobile (to true) and still get the dropdown menu.

Removed the parameter! The last test is to override the default value for all platforms to check if the dropdown menu still works. Just added some more checks in that test to make sure we can make a selection when requestFocusOnTap is true for mobile or requestFocusOnTap is false for desktop.

We should only use firstWidget when more than one widget is expected and the first one is the one we want. Otherwise tester.widget

Presumably variant: TargetPlatformVariant.all() is the default?

Just want to test that the dropdown menu works on all platforms when a value of requestFocusOnTap is provided.

We should verify that the focus doesn't change if the text field is tapped or its context menu is shown (long press or right-button on desktop).

Added more tests.

Setting this field to true or false, rather than allowing the implementation to choose based on the platform, can be useful for applications that want to override the default behavior.

By default, on mobile platforms, tapping on the text field and opening the menu will not case a focus request and the virtual keyboard will not appear. The default behavior for desktop platforms is for the dropdown to take the focus.

Although it doesn't matter very much, this would probably be. slightly better with a short circuit return:

    if (widget.requestFocusOnTap != null) {
      return widget.requestFocusOnTap;
    }
    switch (Theme.of(context).platform) {
      case TargetPlatform.iOS:
      case TargetPlatform.android:
      case TargetPlatform.fuchsia:
        return false
        break;
      case TargetPlatform.macOS:
      case TargetPlatform.linux:
      case TargetPlatform.windows:
        return true;
    }