Skip to content

fix(android): make dialog color options take effect and update API calls #156

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix(android): make dialog color options take effect and update API calls #156

wants to merge 2 commits into from

Conversation

Alexander-Bliznyuk
Copy link

No description provided.

@farfromrefug
Copy link
Member

@Alexander-Bliznyuk thanks!
Could you:

  • store the buttons ids (neutral...) Not to query the native every time
  • why did you change the show() order ? Don't really like this. I am afraid it will have some weird ui effects (like changing while showing)

@Alexander-Bliznyuk
Copy link
Author

  • Yes, I'll extract ids queries to constants a bit later
  • I don't like this style either. Usually we initialize things first and only then build them, especially when it comes to UI. In this case it appears buttons don't exist until dialog is shown, so we get nulls.

@farfromrefug
Copy link
Member

OK then maybe pût only the button part after the show ? With a comment to explain
Thanks

@Alexander-Bliznyuk Alexander-Bliznyuk force-pushed the Alexander-Bliznyuk-android-dialogs-colors branch from e40446f to 0ac7808 Compare August 9, 2020 21:41
@farfromrefug
Copy link
Member

@Alexander-Bliznyuk sorry to come back on this so late. Could you resolve merge conflicts?

@agonper agonper mentioned this pull request Apr 8, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Alexander-Bliznyuk @farfromrefug