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

Replacing alert box with modal ui #5207

Merged
merged 2 commits into from
Aug 11, 2018
Merged

Conversation

vershwal
Copy link
Collaborator

@vershwal vershwal commented Aug 6, 2018

Related #4806 #4977.

@bhousel bhousel merged commit 2d058c7 into openstreetmap:master Aug 11, 2018
@bhousel
Copy link
Member

bhousel commented Aug 11, 2018

Hey @vershwal this is good! I made some changes and merged it..

Here are some of the things I changed:

  • I moved the whole thing into a new subfolder modules/ui/settings. What you built was really good and I realized that we are going to add a bunch more modal settings screens just like this one (for example to customize the vector tile and other data layers). So this gives us a place to organize them.
  • There is another special kind of uiModal called uiConfirm, which already has an OK button, so I used that one. It makes the code a bit simpler..
  • Dispatch a change event instead of trying to call a function back in uiBackground
  • Add utilNoAuto to the textarea (this just prevents autocomplete and other annoying behaviors especially in Safari)

@vershwal
Copy link
Collaborator Author

Thanks, @bhousel!! Just saw the UI, looks great!!
There were many new things involved in this issue...Enjoyed doing this!!! If you find any other similar issue please tag me.

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.

2 participants