Skip to content

Replace standard alert and confirm with BootstrapDialog #501

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
Closed
r-kamath wants to merge 2 commits into from
Closed

Replace standard alert and confirm with BootstrapDialog #501

r-kamath wants to merge 2 commits into from

Conversation

@r-kamath
Copy link
Member

@r-kamath r-kamath commented Dec 2, 2015

Before
screen shot 2015-12-01 at 4 56 45 pm

After
screen shot 2015-12-01 at 4 57 27 pm

@r-kamath r-kamath changed the title [WIP] Replace standard alert and confirm with BootstrapDialog Replace standard alert and confirm with BootstrapDialog Dec 2, 2015
@r-kamath
Copy link
Member Author

r-kamath commented Dec 3, 2015

I was waiting for CI to pass. Ready for review

@prabhjyotsingh
Copy link
Contributor

prabhjyotsingh commented Dec 4, 2015

LGTM, look better than default alerts/confirm box.

@corneadoug
Copy link
Contributor

corneadoug commented Dec 5, 2015

I prefer the browser alert to this one.
The title box is quite useless here

@r-kamath
Copy link
Member Author

r-kamath commented Dec 5, 2015

@corneadoug we can remove the title box if necessary. Even in the standard alert/confirm there is nothing in the title box. IMHO the standard ones are not considered as good UX

@corneadoug
Copy link
Contributor

corneadoug commented Dec 9, 2015

We can always change the style later.
Could you please update the license for BootStrapDialog here:
https://github.com/apache/incubator-zeppelin/blob/b5e2e62f239d6b1b625add16405e3a302e8ff060/zeppelin-distribution/src/bin_license/LICENSE

@corneadoug
Copy link
Contributor

corneadoug commented Dec 9, 2015

Thanks, LGTM

@swkimme
Copy link
Contributor

swkimme commented Dec 9, 2015

Cool! LGTM

2015년 12월 9일 (수) 오후 5:44, CORNEAU Damien notifications@github.com님이 작성:

Thanks, LGTM

—
Reply to this email directly or view it on GitHub
#501 (comment)
.

@Leemoonsoo
Copy link
Member

Leemoonsoo commented Dec 9, 2015

Tested and LGTM.
Merging it into master.

@asfgit asfgit closed this in b358ad2 Dec 9, 2015
@gauravkumar37
Copy link

gauravkumar37 commented Dec 14, 2015

Sorry to barge in at a closed bug.
Neither the cancel nor the OK button has focus on them. Would be nice to have either one pre-focussed.

@r-kamath
Copy link
Member Author

r-kamath commented Dec 15, 2015

Neither the cancel nor the OK button has focus on them. Would be nice to have either one pre-focussed.

@gauravkumar37 thanks for the feedback. I've created a new PR (#532) to address this issue. Will focus OK by default and enable Esc key for cancel popup. PR is currently WIP.

@gauravkumar37
Copy link

gauravkumar37 commented Dec 16, 2015

Thanks @r-kamath, will check that PR out.

@prabhjyotsingh prabhjyotsingh mentioned this pull request Mar 28, 2016
Closed
asfgit pushed a commit that referenced this pull request Mar 31, 2016
@prabhjyotsingh @Leemoonsoo
replace standard alert and confirm with BootstrapDialog
0ee791c 
### What is this PR for?
Replace standard alert and confirm with BootstrapDialog.
Most of these were already take care by #501

### What type of PR is it?
Bug Fix

### What is the Jira issue?
N/A

### How should this be tested?
Try accessing a notebook by user who is not authorized, instead of standard alert there should be a BootstrapDialog.

### Screenshots (if appropriate)
Before:
![screen shot 2016-03-28 at 3 50 59 pm](https://cloud.githubusercontent.com/assets/674497/14076259/d28ee552-f4fd-11e5-80d9-ac648e06c373.png)

After:
![screen shot 2016-03-28 at 3 53 22 pm](https://cloud.githubusercontent.com/assets/674497/14076263/d6410356-f4fd-11e5-8d71-62be584c424f.png)

Author: Prabhjyot Singh <prabhjyotsingh@gmail.com>

Closes #802 from prabhjyotsingh/bootstrapDialog and squashes the following commits:

c561688 [Prabhjyot Singh] replace standard alert and confirm with BootstrapDialog
onkarshedge pushed a commit to onkarshedge/incubator-zeppelin that referenced this pull request May 11, 2016
@prabhjyotsingh @onkarshedge
replace standard alert and confirm with BootstrapDialog
ca35405 
### What is this PR for?
Replace standard alert and confirm with BootstrapDialog.
Most of these were already take care by apache#501

### What type of PR is it?
Bug Fix

### What is the Jira issue?
N/A

### How should this be tested?
Try accessing a notebook by user who is not authorized, instead of standard alert there should be a BootstrapDialog.

### Screenshots (if appropriate)
Before:
![screen shot 2016-03-28 at 3 50 59 pm](https://cloud.githubusercontent.com/assets/674497/14076259/d28ee552-f4fd-11e5-80d9-ac648e06c373.png)

After:
![screen shot 2016-03-28 at 3 53 22 pm](https://cloud.githubusercontent.com/assets/674497/14076263/d6410356-f4fd-11e5-8d71-62be584c424f.png)

Author: Prabhjyot Singh <prabhjyotsingh@gmail.com>

Closes apache#802 from prabhjyotsingh/bootstrapDialog and squashes the following commits:

c561688 [Prabhjyot Singh] replace standard alert and confirm with BootstrapDialog
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.

6 participants

@r-kamath @prabhjyotsingh @corneadoug @swkimme @Leemoonsoo @gauravkumar37