Skip to content

Fixed issues with Background Color #34741 - #39056

Merged
LongCatIsLooong merged 6 commits into
flutter:masterfrom
venkata-ramana:time_picker_background_color_fix
Aug 26, 2019
Merged

Fixed issues with Background Color #34741#39056
LongCatIsLooong merged 6 commits into
flutter:masterfrom
venkata-ramana:time_picker_background_color_fix

Conversation

@venkata-ramana

@venkata-ramana venkata-ramana commented Aug 22, 2019

Copy link
Copy Markdown
Contributor

Description

Added backgroundColor property to CupertinoTimePicker and CupertinoDatePicker.

Related Issues

Issue #34741

@HansMuller HansMuller added p: cupertino_ui cupertino_ui package in flutter/packages framework flutter/packages/flutter repository. See also f: labels. labels Aug 22, 2019
@LongCatIsLooong
LongCatIsLooong self-requested a review August 22, 2019 16:54
LongCatIsLooong
LongCatIsLooong requested changes Aug 22, 2019
View reviewed changes

@LongCatIsLooong LongCatIsLooong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @venkata-ramana thanks for contributing! Could you add a few tests to make sure:

  1. Specifying a backgroundColor works.
  2. If backgroundColor is not specified, the default color will be used.
  3. Changing the backgroundColor works.

Comment thread packages/flutter/lib/src/cupertino/date_picker.dart
Comment thread packages/flutter/lib/src/cupertino/date_picker.dart Outdated
Comment thread packages/flutter/lib/src/cupertino/date_picker.dart
@venkata-ramana

venkata-ramana commented Aug 22, 2019

Copy link
Copy Markdown
Contributor Author

@LongCatIsLooong Added test case coverage and assert checks. Please let me know if anything needs to be done?

LongCatIsLooong
LongCatIsLooong reviewed Aug 22, 2019
View reviewed changes
Comment thread packages/flutter/test/cupertino/date_picker_test.dart Outdated
LongCatIsLooong
LongCatIsLooong reviewed Aug 22, 2019
View reviewed changes
Comment thread packages/flutter/test/cupertino/date_picker_test.dart Outdated
LongCatIsLooong
LongCatIsLooong reviewed Aug 22, 2019
View reviewed changes
Comment thread packages/flutter/test/cupertino/date_picker_test.dart Outdated
@LongCatIsLooong

LongCatIsLooong commented Aug 22, 2019

Copy link
Copy Markdown
Contributor

The analyzer is complaining about trailing spaces:

▌18:36:59▐ RUNNING: cd .; grep --line-number --extended-regexp [[:blank:]]$ packages/flutter/lib/src/cupertino/date_picker.dart packages/flutter/test/cupertino/date_picker_test.dart
packages/flutter/lib/src/cupertino/date_picker.dart:287: ///
packages/flutter/lib/src/cupertino/date_picker.dart:1089:
packages/flutter/lib/src/cupertino/date_picker.dart:1091: ///
▌18:36:59▐ ELAPSED TIME: 0.026s for grep --line-number --extended-regexp [[:blank:]]$ packages/flutter/lib/src/cupertino/date_picker.dart packages/flutter/test/cupertino/date_picker_test.dart in .
Whitespace detected at the end of source code lines.
Please remove:
━━━━━━━━━━━━━━

@venkata-ramana

venkata-ramana commented Aug 23, 2019

Copy link
Copy Markdown
Contributor Author

@LongCatIsLooong updated test cases and fixed trailing spaces issues

LongCatIsLooong
LongCatIsLooong reviewed Aug 23, 2019
View reviewed changes
Comment thread packages/flutter/test/cupertino/date_picker_test.dart Outdated
LongCatIsLooong
LongCatIsLooong approved these changes Aug 24, 2019
View reviewed changes

@LongCatIsLooong LongCatIsLooong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. This is awesome, thanks for the PR!

@LongCatIsLooong
LongCatIsLooong merged commit 19cdb21 into flutter:master Aug 26, 2019
This was referenced Aug 26, 2019
Closed
Closed
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@LongCatIsLooong LongCatIsLooong LongCatIsLooong approved these changes

Assignees

No one assigned

Labels

framework flutter/packages/flutter repository. See also f: labels. p: cupertino_ui cupertino_ui package in flutter/packages

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@venkata-ramana @LongCatIsLooong @HansMuller @googlebot