[material_ui] Add tertiary tap callbacks to InkResponse and InkWell - #12676
[material_ui] Add tertiary tap callbacks to InkResponse and InkWell#12676Ahtsham0715 wants to merge 2 commits into
Conversation
`GestureDetector` exposes `onTertiaryTapDown`, `onTertiaryTapUp` and `onTertiaryTapCancel`, but `InkResponse`/`InkWell` only forwarded the primary and secondary button callbacks. This meant a middle click on an `InkWell` (a common way to open a link in a new tab or window) could not be handled without wrapping the widget in another gesture detector. This adds the three tertiary callbacks, mirroring the existing secondary button plumbing: the ink splash and pressed highlight are started on tertiary tap down and resolved on tap up or cancel, and the widget only listens for the tertiary button when at least one of `onTertiaryTapDown`/`onTertiaryTapUp` is provided, so existing widgets are unaffected. Fixes flutter/flutter#129058
There was a problem hiding this comment.
Code Review
This pull request adds support for tertiary (middle-click) tap gestures (onTertiaryTapDown, onTertiaryTapUp, and onTertiaryTapCancel) to InkResponse and InkWell, along with corresponding tests and a changelog entry. The reviewer suggests expanding the test coverage to verify that the highlight is correctly shown when only onTertiaryTapDown or both tertiary callbacks are provided, rather than only testing onTertiaryTapUp.
| Widget buildFrame({required bool withTertiaryCallback}) { | ||
| return Directionality( | ||
| textDirection: TextDirection.ltr, | ||
| child: Material( | ||
| child: Center( | ||
| child: SizedBox( | ||
| width: 100.0, | ||
| height: 100.0, | ||
| child: InkWell( | ||
| onTap: () {}, | ||
| onHighlightChanged: log.add, | ||
| onTertiaryTapUp: withTertiaryCallback ? (TapUpDetails details) {} : null, | ||
| ), | ||
| ), | ||
| ), | ||
| ), | ||
| ); | ||
| } |
There was a problem hiding this comment.
This test is great for verifying that highlights are shown for tertiary taps only when a callback is provided. However, it only checks for the presence of onTertiaryTapUp. The logic in _tertiaryButtonEnabled also enables tertiary tap handling if onTertiaryTapDown is provided.
To make this test more comprehensive, consider parameterizing it to check that the highlight is correctly shown when:
- Only
onTertiaryTapUpis provided. - Only
onTertiaryTapDownis provided. - Both are provided.
- Neither is provided (as is already tested).
This would provide stronger guarantees about the correctness of the enabling logic.
Ported from flutter/flutter#191971, which was opened against
flutter/flutterbefore I noticed the Material code freeze (flutter/flutter#188444). That PR has been closed in favour of this one.GestureDetectorexposesonTertiaryTapDown,onTertiaryTapUpandonTertiaryTapCancel, butInkResponse/InkWellonly forward the primary and secondary button callbacks. That means a middle click on anInkWell— the usual way to open a link in a new tab or window on desktop and web — can't be handled without wrapping the widget in a second gesture detector, which then has to be reconciled with the ink splash.This PR adds the three tertiary callbacks to
InkResponseandInkWell, mirroring the existing secondary button plumbing:onTertiaryTapDownoronTertiaryTapUpis non-null, so widgets that don't opt in behave exactly as before (no splash on middle click, same as today).onTertiaryTapCancelalone does not enable the button, which matches howonSecondaryTapCancelalready behaves.There is no
onTertiaryTapbecauseTapGestureRecognizer/GestureDetectordon't have one;onTertiaryTapUpis the completion signal for that button.This follows the approach suggested in the issue: a specific widget with a concrete use case, rather than a general reworking of the button callback pattern.
Tests added to
packages/material_ui/test/ink_well_test.dart:InkWell tertiary tap test— verifies the down/up ordering on a completed middle click, and down/cancel when the pointer is dragged away.InkWell tertiary tap highlights only when a tertiary callback is defined— verifies the pressed highlight is raised and lowered on a tertiary press when a tertiary callback is supplied, and that a tertiary press is ignored when one is not.A pending changelog entry is included with a
minorversion bump, since this is additive API.Fixes flutter/flutter#129058
Pre-Review Checklist
[shared_preferences]///).