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

disabled color #177

Merged
merged 6 commits into from
Nov 1, 2017
Merged

disabled color #177

merged 6 commits into from
Nov 1, 2017

Conversation

kanjanaSi
Copy link
Contributor

No description provided.

@k0nserv
Copy link
Contributor

k0nserv commented Oct 26, 2017

Hey @kanjanaSi this looks really neat :) Can you post some screenshots of this as well?

attributedPlaceholder = NSAttributedString(
string: placeholder,
attributes: [
NSAttributedStringKey.foregroundColor: color, NSAttributedStringKey.font: font
Copy link
Contributor

Choose a reason for hiding this comment

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

Please cleanup this indentation to match what it was before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed!

@kanjanaSi
Copy link
Contributor Author

@k0nserv
screen shot 2017-10-26 at 7 52 23 pm

@k0nserv
Copy link
Contributor

k0nserv commented Oct 26, 2017

Awesome :) Would you be up for updating the README.md with documentation and the example project?

@k0nserv
Copy link
Contributor

k0nserv commented Oct 26, 2017

This control is confusing

image

It seems to be doing the opposite for me i.e when it's "disabled" the text fields is actually enabled and vice versa

Copy link
Contributor

@k0nserv k0nserv left a comment

Choose a reason for hiding this comment

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

Awesome, this looks really good. Lastly could we just add some tests for this new behaviour?

@zarzonis
Copy link

@k0nserv Hi Hugo. Do you have any plan to release this? I was ready to implement this feature and I just found out this PR.

@k0nserv
Copy link
Contributor

k0nserv commented Nov 1, 2017

Hey @zarzonis yeah I'll do that right away :)

@k0nserv k0nserv merged commit 109a275 into Skyscanner:master Nov 1, 2017
@k0nserv
Copy link
Contributor

k0nserv commented Nov 1, 2017

That's it released. Thanks a bunch @kanjanaSi for the awesome contribution 🎉

@kanjanaSi
Copy link
Contributor Author

🎉🎉🎉🎉🎉

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.

3 participants