Skip to content

ShaderMask on web for CupertinoContextMenu#77864

Merged
justinmc merged 3 commits intoflutter:masterfrom
justinmc:cupertino-context-menu-shader-mask
Mar 11, 2021
Merged

ShaderMask on web for CupertinoContextMenu#77864
justinmc merged 3 commits intoflutter:masterfrom
justinmc:cupertino-context-menu-shader-mask

Conversation

@justinmc
Copy link
Contributor

@justinmc justinmc commented Mar 11, 2021

I'm told that web now supports ShaderMask, so we can remove this workaround that was put into CupertinoContextMenu to simplify it so it didn't need ShaderMask on web.

Also unskips ShaderMask tests on web.

Closes #52967

@justinmc justinmc self-assigned this Mar 11, 2021
@flutter-dashboard flutter-dashboard bot added f: cupertino flutter/packages/flutter/cupertino repository framework flutter/packages/flutter repository. See also f: labels. labels Mar 11, 2021
@google-cla google-cla bot added the cla: yes label Mar 11, 2021
// conditional and just check for ShaderMask.
// https://github.com/flutter/flutter/issues/52967.
expect(find.byType(ShaderMask), kIsWeb ? findsNothing : findsOneWidget);
expect(find.byType(ShaderMask), findsOneWidget);
Copy link
Contributor

@LongCatIsLooong LongCatIsLooong Mar 11, 2021

Choose a reason for hiding this comment

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

The test is getting skipped when isBrowser is true. Is the skip still needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah good catch, there are a bunch of skipped tests that reference #44152. They seem to pass now, so I will unskip all of them.

@skia-gold
Copy link

Gold has detected about 2 untriaged digest(s) on patchset 3.
View them at https://flutter-gold.skia.org/cl/github/77864

@justinmc justinmc merged commit 216f9b5 into flutter:master Mar 11, 2021
@justinmc justinmc deleted the cupertino-context-menu-shader-mask branch March 11, 2021 18:35
Copy link
Contributor

@ferhatb ferhatb 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 unskipping tests too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

f: cupertino flutter/packages/flutter/cupertino repository framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CupertinoContextMenu doesn't fully work on the web

4 participants