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

add color for selected value as selectedValueColor #69

Merged
merged 9 commits into from
Nov 14, 2020

Conversation

AliRn76
Copy link
Contributor

@AliRn76 AliRn76 commented Apr 5, 2020

I think the color of selected value would be needed most of time.

@coveralls
Copy link

coveralls commented Apr 5, 2020

Pull Request Test Coverage Report for Build 65

  • 9 of 9 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 68.456%

Totals Coverage Status
Change from base Build 59: 0.0%
Covered Lines: 204
Relevant Lines: 298

💛 - Coveralls

@paxcodes
Copy link

I suggest completely passing in a TextStyle for the selectedValue, that way there is complete flexibility of changing the color, fontSize, fontFamily, etc.

And the default style as well.

@AliRn76
Copy link
Contributor Author

AliRn76 commented Jul 14, 2020

I suggest completely passing in a TextStyle for the selectedValue, that way there is complete flexibility of changing the color, fontSize, fontFamily, etc.

And the default style as well.

That would be good , but he wont merge it anyway :)

@MarcinusX
Copy link
Owner

Hey, can we have 2 params in here? One for textStyle and other one for selectedTextStyle? :)
I will merge it then :)

@AliRn76
Copy link
Contributor Author

AliRn76 commented Jul 28, 2020

Done :)
I just set the textStyle and selectedTextStyle for them

@@ -257,10 +269,6 @@ class NumberPicker extends StatelessWidget {
}

Widget _integerListView(ThemeData themeData) {
TextStyle defaultStyle = themeData.textTheme.body1;
Copy link
Owner

Choose a reason for hiding this comment

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

We should still use those styles if user didn't provide any styles.
I believe it go like that:

TextStyle defaultStyle = textStyle ?? themeData.textTheme.body1;
TextStyle selectedStyle =
       selectedTextStyle ?? themeData.textTheme.headline.copyWith(color: themeData.accentColor);

Same in other ocurrences

Copy link
Owner

Choose a reason for hiding this comment

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

Also, could you undo all new formatting? :) So that in one PR we can only see those changes that are important.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sry for that format problem :\
I didn't change them , this ide decide to change them :(
btw I fix them

Choose a reason for hiding this comment

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

any news on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's complete, we waiting for accepting the pull request :)

@paxcodes
Copy link

Thanks for merging @MarcinusX ! 🎉

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

Successfully merging this pull request may close these issues.

5 participants