this code doesn't register to be told when the flag changes, which seems bad. I would recommend putting this flag in the MediaQueryData object and having the caller read that then pass in the flag to this method.

you should also check that changing the state of the flag changes the actual rendered widget

is this expected?

Should be gone now (added the module in a different PR).

rather than "and it has", consider "which has", since you're talking about MediaQueryData.alwaysUse24HourFormat itself rather than the value of this argument or the behaviour of this method.

Done.

if you feel it is important to link to MediaQueryData.alwaysUse24HourFormat twice, consider the more common "see also" style, as in:

  /// See also:
  ///
  ///  * [MediaQueryData.alwaysUse24HourFormat], the documentation for which provides more detail.

...or something.

(or leave it as you have it now if you think that's clearer)

Given the first link, the last sentence is redundant. Removed it.

you can remove this else

Done.

interesting that timeOfDay.hourOfPeriod doesn't do this logic for you.

Doesn't seem like it does. It is defined as:

 int get hourOfPeriod => hour - periodOffset;

where hour is 0..23, and periodOffset is either 0 or 12. So the result is 0-11, rather than 12-1-11.

Actually, this logic existed before. I think it got lost during the flutter_localizations split. A-ha!

remove return

Done (removed else)

assert number is 0..99

Done.

if you cannot handle MediaQuery.of() returning null (which seems reasonable, many material widgets require this) then please add an assert to the build method that calls debugCheckHasMediaQuery. (See other widgets for examples of doing this.)

Done.

ditto

Done.

??

That's how the dials behave on Android, the AM dial's top position corresponds to noon (12:00), while the PM dial's top position corresponds to midnight (00:00). We had it reversed.

ditto assert

Done.

how does is24h interact with the new flag?

It seems weird to be able to have the double ring with the 12h mode or the single ring with 24h mode...

Good points:

how does is24h interact with the new flag?

This option is only used to track whether we need to use one dial (12h) or two dials (24h). So I renamed it to use24HourDials.

It seems weird to be able to have the double ring with the 12h mode or the single ring with 24h mode.

Theoretically an implementor of a custom MaterialLocalizations could end up with a 12h/24h mix if the way they determine the 24-hour-formattedness of time was different from how the time picker does it (e.g. if they chose to not honor the 24-hour setting). So, I changed it so that MaterialLocalizations.timeOfDayFormat, which now takes alwaysUse24HourFormat, is the single source of truth of 24-hour-formattedness of time. The value of use24HourDials is now computed purely from MaterialLocalizations.timeOfDayFormat.

i was a bit confused when reading this because I thought i interpreted "_initFoo" as methods that get called from initState. Maybe rename those buildFoo to make it clearer they are called every build?

Done.

do we only expose this for testing?

seems weird to expose a TimePickerDialog but not a DatePickerDialog.
seems weird to expose either if they're an implementation detail of showTimePicker et al.

Done (used custom Navigator/MediaQuery/Localizations/Directionality).

avoid backticks around true, false, and null.

Done.

see earlier question

Reverted. This was added in a separate PR.

you can write this test with showTimePicker, use a GlobalKey to get the context from inside the MediaQuery.

Done (used custom Navigator/MediaQuery/Localizations/Directionality).