Sitelet https://web.archive.org/web/20201130085748/https://github.com/flutter/flutter/pull/71380
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

Card respects surface color #71380

Open
wants to merge 1 commit into
base: master
from

Conversation

@Amitpatil215
Copy link
Contributor

@Amitpatil215 Amitpatil215 commented Nov 29, 2020

Description

Existing Behaviour
Card takes default white color even if surface color is not-null.

Modified Behaviour
If surface color is given then card respects the surface color else it takes default white color.

Runnable Code Snippets

MyApp.dart

class MyApp extends StatelessWidget {
  static ColorScheme colorScheme = ColorScheme.light().copyWith(
    surface: Colors.purple,
  );
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      theme: ThemeData(
        colorScheme: colorScheme,
      ),
      home: Scaffold(
        body: Center(
          child: Card(
            child: Text(
              "Card",
              style: TextStyle(fontSize: 20),
            ),
          ),
        ),
      ),
    );
  }
}

Related Issues

fixes #28783

Tests

I added the following tests:

  • Test written for verifying card applies surface color if Color property in Card, [CardTheme.color] and [ThemeData.cardColor] not given respectively.
  • This test written forColorScheme.light().copyWith() , but this implementation remains true for ColorScheme.dark() , ColorScheme() . Should I write test for all of them?

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

  • No, no existing tests failed, so this is not a breaking change.
@Amitpatil215
Copy link
Contributor Author

@Amitpatil215 Amitpatil215 commented Nov 30, 2020

@shihaohong would be great if you could review my PR

@shihaohong
Copy link
Contributor

@shihaohong shihaohong commented Nov 30, 2020

This would be a pretty breaking change for theming Cards, since it'll default to surface color over the previous default.

/cc @HansMuller, who's been working a lot more with themes lately than I have, but my suspicion is that we should not making this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.