Skip to content
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

[Ink] Conditionally maskToBounds on MDCInkView layer #9818

Merged

Conversation

andrewoverton
Copy link
Contributor

@andrewoverton andrewoverton commented Feb 28, 2020

This change makes it so that the masksToBounds property on an MDCButton's MDCInkView's MDCLegacyInkLayer is set according to whether inkStyle is .unbounded or not, regardless of whether usesLegacyInkRipple is set on the MDCButton.

An MDCInkView's layer is always an MDCLegacyInkLayer, even when usesLegacyInkRipple is set to NO. In this scenario it acts as the MDCInkView's layer, even if it's not acting as the thing that triggers the ink/ripple animation.

There was a very similar bug to this recently that #9254 was intended to address. That PR addressed it in the Catalog, as you can see in the before and after gifs in the PR description, but a different client was still experiencing the issue. I built the second client's app to test this PR and it seems to fix it for them.

I wonder if we should try to get rid of MDCLegacyInkLayer, or if we should bypass that and just get rid of MDCInkView entirely, now that we have MDCRippleView.

Closes #9762.

@material-automation material-automation bot changed the title Conditionally maskToBounds on MDCInkView layer [Ink] Conditionally maskToBounds on MDCInkView layer Feb 28, 2020
@material-automation
Copy link

Based on the changes, the title has been prefixed with [Ink].

@material-automation
Copy link

material-automation commented Feb 28, 2020

bazel detected changes to the following targets:

//components/ActionSheet:snapshot_tests
//components/ActionSheet:snapshot_tests_test_bundle
//components/ActionSheet:unit_tests
//components/ActionSheet:unit_tests_IPAD_PRO_12_9_IN_9_3
//components/ActionSheet:unit_tests_IPAD_PRO_12_9_IN_9_3_test_bundle
//components/ActionSheet:unit_tests_IPHONE_7_PLUS_IN_10_3
//components/ActionSheet:unit_tests_IPHONE_7_PLUS_IN_10_3_test_bundle
//components/ActionSheet:unit_tests_IPHONE_X_IN_11_0
//components/ActionSheet:unit_tests_IPHONE_X_IN_11_0_test_bundle
//components/AppBar:snapshot_tests
//components/AppBar:snapshot_tests_test_bundle
//components/AppBar:unit_tests
//components/AppBar:unit_tests_IPAD_PRO_12_9_IN_9_3
//components/AppBar:unit_tests_IPAD_PRO_12_9_IN_9_3_test_bundle
//components/AppBar:unit_tests_IPHONE_7_PLUS_IN_10_3
//components/AppBar:unit_tests_IPHONE_7_PLUS_IN_10_3_test_bundle
//components/AppBar:unit_tests_environment
//components/AppBar:unit_tests_environment_test_bundle
//components/Banner:snapshot_tests
//components/Banner:snapshot_tests_test_bundle
//components/Banner:unit_tests
//components/Banner:unit_tests_IPAD_PRO_12_9_IN_9_3
//components/Banner:unit_tests_IPAD_PRO_12_9_IN_9_3_test_bundle
//components/Banner:unit_tests_IPHONE_7_PLUS_IN_10_3
//components/Banner:unit_tests_IPHONE_7_PLUS_IN_10_3_test_bundle
//components/Banner:unit_tests_environment
//components/Banner:unit_tests_environment_test_bundle
//components/BottomAppBar:snapshot_tests
//components/BottomAppBar:snapshot_tests_test_bundle
//components/BottomAppBar:unit_tests
//components/BottomAppBar:unit_tests_IPAD_PRO_12_9_IN_9_3
//components/BottomAppBar:unit_tests_IPAD_PRO_12_9_IN_9_3_test_bundle
//components/BottomAppBar:unit_tests_IPHONE_7_PLUS_IN_10_3
//components/BottomAppBar:unit_tests_IPHONE_7_PLUS_IN_10_3_test_bundle
//components/BottomAppBar:unit_tests_environment
//components/BottomAppBar:unit_tests_environment_test_bundle
//components/BottomNavigation:snapshot_tests
//components/BottomNavigation:snapshot_tests_test_bundle
//components/BottomNavigation:unit_tests
//components/BottomNavigation:unit_tests_IPAD_PRO_12_9_IN_9_3
//components/BottomNavigation:unit_tests_IPAD_PRO_12_9_IN_9_3_test_bundle
//components/BottomNavigation:unit_tests_IPHONE_7_PLUS_IN_10_3
//components/BottomNavigation:unit_tests_IPHONE_7_PLUS_IN_10_3_test_bundle
//components/BottomNavigation:unit_tests_beta
//components/BottomNavigation:unit_tests_beta_test_bundle
//components/BottomNavigation:unit_tests_environment
//components/BottomNavigation:unit_tests_environment_test_bundle
//components/ButtonBar:snapshot_tests
//components/ButtonBar:snapshot_tests_test_bundle
//components/ButtonBar:unit_tests
//components/ButtonBar:unit_tests_IPAD_PRO_12_9_IN_9_3
//components/ButtonBar:unit_tests_IPAD_PRO_12_9_IN_9_3_test_bundle
//components/ButtonBar:unit_tests_IPHONE_7_PLUS_IN_10_3
//components/ButtonBar:unit_tests_IPHONE_7_PLUS_IN_10_3_test_bundle
//components/ButtonBar:unit_tests_IPHONE_X_IN_11_0
//components/ButtonBar:unit_tests_IPHONE_X_IN_11_0_test_bundle
//components/Buttons:snapshot_tests
//components/Buttons:snapshot_tests_test_bundle
//components/Buttons:unit_tests
//components/Buttons:unit_tests_IPAD_PRO_12_9_IN_9_3
//components/Buttons:unit_tests_IPAD_PRO_12_9_IN_9_3_test_bundle
//components/Buttons:unit_tests_IPHONE_7_PLUS_IN_10_3
//components/Buttons:unit_tests_IPHONE_7_PLUS_IN_10_3_test_bundle
//components/Buttons:unit_tests_environment
//components/Buttons:unit_tests_environment_test_bundle
//components/Cards:snapshot_tests
//components/Cards:snapshot_tests_test_bundle
//components/Cards:unit_tests
//components/Cards:unit_tests_IPAD_PRO_12_9_IN_9_3
//components/Cards:unit_tests_IPAD_PRO_12_9_IN_9_3_test_bundle
//components/Cards:unit_tests_IPHONE_7_PLUS_IN_10_3
//components/Cards:unit_tests_IPHONE_7_PLUS_IN_10_3_test_bundle
//components/Cards:unit_tests_environment
//components/Cards:unit_tests_environment_test_bundle
//components/Chips:snapshot_tests
//components/Chips:snapshot_tests_test_bundle
//components/Chips:unit_tests
//components/Chips:unit_tests_IPAD_PRO_12_9_IN_9_3
//components/Chips:unit_tests_IPAD_PRO_12_9_IN_9_3_test_bundle
//components/Chips:unit_tests_IPHONE_7_PLUS_IN_10_3
//components/Chips:unit_tests_IPHONE_7_PLUS_IN_10_3_test_bundle
//components/Chips:unit_tests_environment
//components/Chips:unit_tests_environment_test_bundle
//components/CollectionCells:unit_tests
//components/CollectionCells:unit_tests_IPAD_PRO_12_9_IN_9_3
//components/CollectionCells:unit_tests_IPAD_PRO_12_9_IN_9_3_test_bundle
//components/CollectionCells:unit_tests_IPHONE_7_PLUS_IN_10_3
//components/CollectionCells:unit_tests_IPHONE_7_PLUS_IN_10_3_test_bundle
//components/CollectionCells:unit_tests_environment
//components/CollectionCells:unit_tests_environment_test_bundle
//components/Collections:unit_tests
//components/Collections:unit_tests_IPAD_PRO_12_9_IN_9_3
//components/Collections:unit_tests_IPAD_PRO_12_9_IN_9_3_test_bundle
//components/Collections:unit_tests_IPHONE_7_PLUS_IN_10_3
//components/Collections:unit_tests_IPHONE_7_PLUS_IN_10_3_test_bundle
//components/Collections:unit_tests_environment
//components/Collections:unit_tests_environment_test_bundle
//components/Dialogs:snapshot_tests
//components/Dialogs:snapshot_tests_test_bundle
//components/Dialogs:unit_tests
//components/Dialogs:unit_tests_IPAD_PRO_12_9_IN_9_3
//components/Dialogs:unit_tests_IPAD_PRO_12_9_IN_9_3_test_bundle
//components/Dialogs:unit_tests_IPHONE_7_PLUS_IN_10_3
//components/Dialogs:unit_tests_IPHONE_7_PLUS_IN_10_3_test_bundle
//components/Dialogs:unit_tests_environment
//components/Dialogs:unit_tests_environment_test_bundle
//components/Ink:snapshot_tests
//components/Ink:snapshot_tests_test_bundle
//components/Ink:unit_tests
//components/Ink:unit_tests_IPAD_PRO_12_9_IN_9_3
//components/Ink:unit_tests_IPAD_PRO_12_9_IN_9_3_test_bundle
//components/Ink:unit_tests_IPHONE_7_PLUS_IN_10_3
//components/Ink:unit_tests_IPHONE_7_PLUS_IN_10_3_test_bundle
//components/Ink:unit_tests_environment
//components/Ink:unit_tests_environment_test_bundle
//components/List:snapshot_tests
//components/List:snapshot_tests_test_bundle
//components/List:unit_tests
//components/List:unit_tests_IPAD_PRO_12_9_IN_9_3
//components/List:unit_tests_IPAD_PRO_12_9_IN_9_3_test_bundle
//components/List:unit_tests_IPHONE_7_PLUS_IN_10_3
//components/List:unit_tests_IPHONE_7_PLUS_IN_10_3_test_bundle
//components/List:unit_tests_environment
//components/List:unit_tests_environment_test_bundle
//components/NavigationBar:snapshot_tests
//components/NavigationBar:snapshot_tests_test_bundle
//components/NavigationBar:unit_tests
//components/NavigationBar:unit_tests_IPAD_PRO_12_9_IN_9_3
//components/NavigationBar:unit_tests_IPAD_PRO_12_9_IN_9_3_test_bundle
//components/NavigationBar:unit_tests_IPHONE_7_PLUS_IN_10_3
//components/NavigationBar:unit_tests_IPHONE_7_PLUS_IN_10_3_test_bundle
//components/NavigationBar:unit_tests_IPHONE_X_IN_11_0
//components/NavigationBar:unit_tests_IPHONE_X_IN_11_0_test_bundle
//components/ShadowElevations:snapshot_tests
//components/ShadowElevations:snapshot_tests_test_bundle
//components/Slider:snapshot_tests
//components/Slider:snapshot_tests_test_bundle
//components/Slider:unit_tests
//components/Slider:unit_tests_IPAD_PRO_12_9_IN_9_3
//components/Slider:unit_tests_IPAD_PRO_12_9_IN_9_3_test_bundle
//components/Slider:unit_tests_IPHONE_7_PLUS_IN_10_3
//components/Slider:unit_tests_IPHONE_7_PLUS_IN_10_3_test_bundle
//components/Slider:unit_tests_environment
//components/Slider:unit_tests_environment_test_bundle
//components/Snackbar:snapshot_tests
//components/Snackbar:snapshot_tests_test_bundle
//components/Snackbar:unit_tests
//components/Snackbar:unit_tests_IPAD_PRO_12_9_IN_9_3
//components/Snackbar:unit_tests_IPAD_PRO_12_9_IN_9_3_test_bundle
//components/Snackbar:unit_tests_IPHONE_7_PLUS_IN_10_3
//components/Snackbar:unit_tests_IPHONE_7_PLUS_IN_10_3_test_bundle
//components/Snackbar:unit_tests_environment
//components/Snackbar:unit_tests_environment_test_bundle
//components/Tabs:snapshot_tests
//components/Tabs:snapshot_tests_test_bundle
//components/Tabs:unit_tests
//components/Tabs:unit_tests_IPAD_PRO_12_9_IN_9_3
//components/Tabs:unit_tests_IPAD_PRO_12_9_IN_9_3_test_bundle
//components/Tabs:unit_tests_IPHONE_7_PLUS_IN_10_3
//components/Tabs:unit_tests_IPHONE_7_PLUS_IN_10_3_test_bundle
//components/Tabs:unit_tests_environment
//components/Tabs:unit_tests_environment_test_bundle
//components/TextFields:snapshot_tests
//components/TextFields:snapshot_tests_test_bundle
//components/TextFields:unit_tests
//components/TextFields:unit_tests_IPAD_PRO_12_9_IN_9_3
//components/TextFields:unit_tests_IPAD_PRO_12_9_IN_9_3_test_bundle
//components/TextFields:unit_tests_IPHONE_7_PLUS_IN_10_3
//components/TextFields:unit_tests_IPHONE_7_PLUS_IN_10_3_test_bundle
//components/TextFields:unit_tests_IPHONE_X_IN_11_0
//components/TextFields:unit_tests_IPHONE_X_IN_11_0_test_bundle
//components/private/ThumbTrack:unit_tests
//components/private/ThumbTrack:unit_tests_IPAD_PRO_12_9_IN_9_3
//components/private/ThumbTrack:unit_tests_IPAD_PRO_12_9_IN_9_3_test_bundle
//components/private/ThumbTrack:unit_tests_IPHONE_7_PLUS_IN_10_3
//components/private/ThumbTrack:unit_tests_IPHONE_7_PLUS_IN_10_3_test_bundle
//components/private/ThumbTrack:unit_tests_environment
//components/private/ThumbTrack:unit_tests_environment_test_bundle

@andrewoverton andrewoverton merged commit 5862e5b into material-components:develop Mar 2, 2020
@andrewoverton andrewoverton deleted the issue-9762 branch March 2, 2020 06:21
andrewoverton added a commit that referenced this pull request Mar 10, 2020
… layer and max ripple radius, ink style, etc (#9855)

This PR aims to simplify the logic that grapples with the interplay of usesLegacyInkLayer with properties like maxRippleRadius and inkStyle. I recently merged #9818 to address this issue, and while I believe it fixes his immediate issue it still leaves other potential issues. Specifically, it does not resolve potential problems related to the ordering of calls to setters for `usesLegacyInkLayer`, `inkStyle`, and `maxRippleRadius` impacting their effectiveness. Hopefully this PR addresses those issues without breaking anything else. I tried to manually test it pretty rigorously by setting all the properties in question in different orders and making sure that everything worked how I expected it to.

Closes #9762.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Buttons] Internal issue: b/149409642
4 participants