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

Provided default values for when user set's nil as color for tab bar items. #908

Merged
merged 2 commits into from
Oct 14, 2017

Conversation

mohpor
Copy link
Contributor

@mohpor mohpor commented Oct 14, 2017

Regarding #860 , #904
This solves the problem of overridden prepare() functions and you can set tabItemsNormalTitleColor and tabItemsSelectedTitleColor at any time.

Copy link
Member

@daniel-jonathan daniel-jonathan left a comment

Choose a reason for hiding this comment

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

Looks good :)

@daniel-jonathan daniel-jonathan merged commit 543012f into CosmicMind:development Oct 14, 2017
@daniel-jonathan daniel-jonathan self-assigned this Oct 15, 2017
@daniel-jonathan
Copy link
Member

I removed this merge and the other ones prior in relation to this. The fix doesn't consider images.

@mohpor
Copy link
Contributor Author

mohpor commented Oct 15, 2017

Could've built on these, but ok.

@daniel-jonathan
Copy link
Member

Yes and no. There are properties now that will need to be maintained, and I think it would be better to do something like this:

tabBar.setColor(.blue, for: .selected)

This would completely abstract any specific attribute this is set for (images, text) and removes the management of the properties internally.

@daniel-jonathan
Copy link
Member

Please find this feature available in the latest Material 2.12.0 release.

Example:

tabBar.setColor(Color.grey.base, for: .normal)
tabBar.setColor(Color.blue.base, for: .selected)

Thank you @mohpor for the initial version, it really helped with laying down the groundwork :)

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

Successfully merging this pull request may close these issues.

3 participants