add support to customize Slider interacivity

[werainkhatri]

design doc: https://docs.flutter.dev/go/permissible-slider-interaction

closes #113370

open questions:

• Should this be added to debugFillProperties? I see only a few things existing in that list.
• Any semantic config change needed? Now that SliderInteraction.none is handled differently than onChanged being null.
  • No, as SliderInteraction.none doesn't exist anymore.
• Should Slider.allowedInteraction be added to SliderTheme too?
  • Yes (done)
• Should SliderInteraction.slideThumb use the actual thumb rect or overlay rect to determine if the user has tapped down on the “thumb”?
• Should SliderInteraction.slideOnTrack be renamed to SliderInteraction.slideOnly coz its essentially that?
  • Yes.
• Any other name suggestions for -?
  • The enum
    • SliderInteraction
    • SliderAction
  • Slider property
    • Slider.allowedInteraction
    • Slider.permissibleInteraction
    • Slider.interaction
    • Slider.allowedAction
    • Slider.permittedAction

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.

[Piinks]

Normally we use switch statements when working with enums. There is some reasoning on this in the style guide:

https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#avoid-using-if-chains-or--or--with-enum-values

[werainkhatri]

ohh that's interesting, thanks for pointing that out.

EDIT: after looking into it, i realized that i'm not examining the enum at one place, rather evaluating different values in different places depending on the usecase. i can move the == checks to the if statements below if that'll make it more consistent with the style guide. lemme know your thoughts.

[Piinks]

The if/else statements should really be converted to switch statements. That way it is clear what is/isn't happening for each type of interaction, and more importantly, this will ensure the analyzer warns us if we ever change the values of the enum

[werainkhatri]

done ✅, PTAL.

[Piinks]

@TahaTesser may also have some thoughts on this

[TahaTesser]

@werainkhatri
Are you planning to update this to include the switch statement?s

[werainkhatri]

Are you planning to update this to include the switch statement?s

yes! have been busy with work. will do it soon.

[TahaTesser]

@werainkhatri
Just tried the latest commit. Overall this looks good.
Could you please update your description to reflect the progress and remove anything not relevant anymore?

Should Slider.allowedInteraction be added to SliderTheme too?

I don't see a reason to not add this to the SliderTheme. Would be nice to theme slider interactions.

[werainkhatri]

thank you so much! glad to hear that. will do the same by the weekend as I'm travelling now.

[Piinks]

This looks very close! Can you update the docs a bit more?

[werainkhatri]

thanks for the review, and fixing the style.

could you help me with this from the description?

• Should this be added to debugFillProperties? I see only a few things existing in that list.

also, are the names of classes and arguments okay?

[TahaTesser]

thanks for the review, and fixing the style.

could you help me with this from the description?

• Should this be added to debugFillProperties? I see only a few things existing in that list.

also, are the names of classes and arguments okay?

You can add them as it is, we can always edit the names before finalizing this.

[TahaTesser]

Suggested change

	expect(value, expectYesOrNo ? equals(0.5) : isNot(equals(0.5)),
	reason: 'allowedInteraction is $whileTesting');
	expect(value, expectYesOrNo ? equals(0.5) : isNot(equals(0.5)),
	reason: 'allowedInteraction is $whileTesting');

This is a bit hard to read.

[Piinks]

I agree. The expectYesOrNo is difficult to follow. It is ok for tests to be a bit more verbose and written out for each test case.

[werainkhatri]

yeah that makes sense. sorry about that. i went overboard with the DRY principle, but at the cost of readability. improved it now.

[Piinks]

Looks like there is just some semantic refactoring in the tests left to do here. Thanks very much for your patience on this!