Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Fix remaining iconbuttontheme overrides in listtile #169029

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
Loading
from

Conversation

pogojotz
Copy link
Contributor

@pogojotz pogojotz commented May 17, 2025

Fix remaining outstanding IconButtonTheme overrides: overlayColor and mouseCursor.

Issue: #167727

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels May 17, 2025
@flutter-dashboard
Copy link

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

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

Changes reported for pull request #169029 at sha 3023df5

@flutter-dashboard flutter-dashboard bot added the will affect goldens Changes to golden files label May 17, 2025
@pogojotz pogojotz mentioned this pull request May 18, 2025
8 tasks
@dkwingsmt dkwingsmt requested a review from QuncCccccc May 21, 2025 18:17
Copy link
Contributor

@QuncCccccc QuncCccccc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for your contribution! This PR #168480 also fixes the same issue. Seem this PR is duplicated:) Are there any cases that the PR #168480 doesn't cover?

@pogojotz
Copy link
Contributor Author

This is not a duplicate. It builds upon #168480. With the changes in #168480 overlayColor and mouseCursor would still be overriden. This is fixing that and extends the test accordingly.

Copy link
Contributor

@QuncCccccc QuncCccccc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see why the overlayColor and mouseCursor don't work. Nice catch!

For mouseCursor, I think the problem is located in the IconButton.styleFrom() in icon_button.dart. In the method, we should check whether both disabledMouseCursor and enabledMouseCursor are null; if yes, then we should just assign null to mouseCursor; otherwise, keep the original implementation.

styleFrom(
...
  mouseCursor: disabledMouseCursor == null && enabledMouseCursor == null
              ? null
              : WidgetStateProperty<MouseCursor?>.fromMap(
                <WidgetStatesConstraint, MouseCursor?>{
                  WidgetState.disabled: disabledMouseCursor,
                  WidgetState.any: enabledMouseCursor,
                },
              ),
...
);

For the overlayColor, styleFrom will generate overlayColor for IconButton based on the foreground color. So the IconButtonTheme overlayColor can never successfully merge into IconButton.styleFrom(). Nice catch!

@pogojotz pogojotz requested a review from QuncCccccc May 23, 2025 21:16
packages/flutter/lib/src/material/list_tile.dart Outdated Show resolved Hide resolved
@pogojotz pogojotz force-pushed the fix-remaining-iconbuttontheme-overrides-in-listtile branch from f14f1d8 to 756008f Compare May 31, 2025 23:31
@pogojotz pogojotz force-pushed the fix-remaining-iconbuttontheme-overrides-in-listtile branch from 5ee5a1e to 9c15bc2 Compare June 5, 2025 21:04
Copy link
Contributor

@QuncCccccc QuncCccccc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM:) Thank you for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. will affect goldens Changes to golden files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.