rydmike
commented
8 months ago Hi @geovaniprodata, this indeed a correct observation of minor bug. Good find, thank you! 😄
It has probably been left unnoticed for this long since very few actually uses the convenience function showColorPickerDialog. It should of course be fixed.
As you can see, your PR fails checks since it does not use dart format, which allows only max width 80 chars. Which is needed for packages on pub. Pub scoring check that packages uses dart format, and only with 80 chars line length, it does not understand anything else. If packages on pub do not pass dart format check loose some score points. Yes 80 chars is a silly low number for Flutter, I agree, but it it was pub requires for best score and there is no way to enforce other than 80 chars on project level.
Usually I would not approve a PR that has a lot of changes in updated file not related to the fix, one previously omitted line in this case. But since I can see it is the only thing that has actually been changed, I am going to merge it anyway.
The placement is also wrong, it should come after the subheading. I'm not going to nitpick about that either, I'll move it after merging.
Also need to create an issue for the PR that it fixes.
We should add tests for this particular dialog function and prop too, but that is something is a bit lacking anyway. I think I have the test for the ColorPicker that does it, but not via this showColorPickerDialog so I will add a case for that too.
You guys forgot to insert tonalSubheading when use show color picker dialog, the ColorPicker itself use the parameter, but the dialog only declares him.