It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

To reconfirm, if canTapOnHeader is true, is there no way to trigger the icon splash at all?

Also, can you add tests?

@dkwingsmt Correct. The Icon shouldn't splash if canTapOnHeader is true, as the whole panel splashes, so it would look too busy. It is more of a signal to the user that the panel is expandable than a button, if canTapOnHeader is true.

I will add tests.

@dkwingsmt I have implemented the changes, I noticed highlightColor was available for InkWell and IconButton so I just added them both, it is a bit clearer than setting the IconButton.highlightColor to ExpansionPanel.splashColor.

I might have exposed a bug with IconButton.splashColor. We don't see the blue it in the below example, but the widget test that checks the IconButton.splashColor passes. It will be why I was setting the IconButton.highlightColor in my first commit.

You can test with this in main.dart:

import 'package:flutter/material.dart';

/// Flutter code sample for [ExpansionPanelList].

void main() => runApp(const ExpansionPanelListExampleApp());

class ExpansionPanelListExampleApp extends StatelessWidget {
  const ExpansionPanelListExampleApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: Scaffold(
        appBar: AppBar(title: const Text('ExpansionPanelList Sample')),
        body: const ExpansionPanelListExample(),
      ),
    );
  }
}

// stores ExpansionPanel state information
class Item {
  Item({
    required this.expandedValue,
    required this.headerValue,
    required this.value,
    required this.canTapOnHeader,
  });

  String expandedValue;
  String headerValue;
  int value;
  bool canTapOnHeader;
}

List<Item> generateItems(int numberOfItems) {
  return List<Item>.generate(numberOfItems, (int index) {
    bool canTapOnHeader = index > 3;
    return Item(
      headerValue: 'Panel $index canTapOnHeader=${canTapOnHeader.toString()}',
      expandedValue: 'This is item number $index',
      value: index,
      canTapOnHeader: canTapOnHeader,
    );
  });
}

class ExpansionPanelListExample extends StatefulWidget {
  const ExpansionPanelListExample({super.key});

  @override
  State<ExpansionPanelListExample> createState() =>
      _ExpansionPanelListExampleState();
}

class _ExpansionPanelListExampleState extends State<ExpansionPanelListExample> {
  final List<Item> _data = generateItems(8);

  @override
  Widget build(BuildContext context) {
    return SingleChildScrollView(
      child: Container(
        child: _buildPanel(),
      ),
    );
  }

  Widget _buildPanel() {
    return ExpansionPanelList.radio(
      children: _data.map<ExpansionPanelRadio>((Item item) {
        return ExpansionPanelRadio(
          splashColor: Colors.blue,
          highlightColor: Colors.green,
          headerBuilder: (BuildContext context, bool isExpanded) {
            return ListTile(
              title: Text(item.headerValue),
            );
          },
          body: ListTile(
              title: Text(item.expandedValue),
          ),
          value: item.value,
          canTapOnHeader: item.canTapOnHeader,
        );
      }).toList(),
      expandIconColor: Colors.red,
    );
  }
}

Hi @dkwingsmt, can we progress this? It should be visibly working by running the example code in my above comment, and the widget tests verify the colors get passed through to the underlying widgets.

The change looks great!

I also ran the demo above, and I see that in the canTapOnHeader=false cases, there are only green splashes, and no blue highlights, as opposed to the canTapOnHeader=true cases. Does it mean that IconButton.highlightColor is never applied?

Thanks @dkwingsmt. I have updated the test to verify that the underlying IconButton.splashColor and IconButton.highlightColor are both correctly set to the ExpansionPanel equivalent fields; ExpansionPanel.splashColor and ExpansionPanel.highlightColor, as this is the widget that will display the button splashes.

There was some confusion around the fact that IconButton.splashColor is unused in material 3. From the IconButton.splashColor code comments:

If [ThemeData.useMaterial3] is set to true, this will not be used. Use [highlightColor] instead to show the overlay color of the button when the button is in the pressed state.

However, Inkwell does use splashColor in Material 3, so both properties are useful, no matter which version of Material theme is used.

I have updated the doc comments to inform the developer when highlightColor should be used instead of splashColor.

Cheers @dkwingsmt & @justinmc. Good to merge?