You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by r-kamath <gi...@git.apache.org> on 2015/12/02 07:03:47 UTC
[GitHub] incubator-zeppelin pull request: [WIP] Replace standard alert and ...
GitHub user r-kamath opened a pull request:
https://github.com/apache/incubator-zeppelin/pull/501
[WIP] Replace standard alert and confirm with BootstrapDialog
Before
<img width="566" alt="screen shot 2015-12-01 at 4 56 45 pm" src="https://cloud.githubusercontent.com/assets/2031306/11523424/303e835a-98e8-11e5-9f97-c4618b2905ee.png">
<hr />
After
<img width="789" alt="screen shot 2015-12-01 at 4 57 27 pm" src="https://cloud.githubusercontent.com/assets/2031306/11523423/3038205a-98e8-11e5-95b5-90983f6ae323.png">
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/r-kamath/incubator-zeppelin bootstrapDialog
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/501.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #501
----
commit 8d18e2dbf7053d934124eed00fe5d4a78e79fb7a
Author: Renjith Kamath <re...@gmail.com>
Date: 2015-12-02T05:58:20Z
replace standard alert and confirm with BootstrapDialog
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: Replace standard alert and confir...
Posted by gauravkumar37 <gi...@git.apache.org>.
Github user gauravkumar37 commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/501#issuecomment-165162392
Thanks @r-kamath, will check that PR out.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: Replace standard alert and confir...
Posted by swkimme <gi...@git.apache.org>.
Github user swkimme commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/501#issuecomment-163154333
Cool! LGTM
2015년 12월 9일 (수) 오후 5:44, CORNEAU Damien <no...@github.com>님이 작성:
> Thanks, LGTM
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/incubator-zeppelin/pull/501#issuecomment-163151792>
> .
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: Replace standard alert and confir...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/501#issuecomment-163379597
Tested and LGTM.
Merging it into master.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: Replace standard alert and confir...
Posted by r-kamath <gi...@git.apache.org>.
Github user r-kamath commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/501#issuecomment-162218643
@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
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: Replace standard alert and confir...
Posted by r-kamath <gi...@git.apache.org>.
Github user r-kamath commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/501#issuecomment-161613487
I was waiting for CI to pass. Ready for review
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: Replace standard alert and confir...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-zeppelin/pull/501
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: Replace standard alert and confir...
Posted by r-kamath <gi...@git.apache.org>.
Github user r-kamath commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/501#issuecomment-164659614
> 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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: Replace standard alert and confir...
Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/501#issuecomment-163110123
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
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: Replace standard alert and confir...
Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/501#issuecomment-162160019
I prefer the browser alert to this one.
The title box is quite useless here
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: Replace standard alert and confir...
Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/501#issuecomment-163151792
Thanks, LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: Replace standard alert and confir...
Posted by gauravkumar37 <gi...@git.apache.org>.
Github user gauravkumar37 commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/501#issuecomment-164472215
Sorry to barge in a closed bug.
Neither the cancel nor the OK button has focus on them. Would be nice to have either pre-focussed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: Replace standard alert and confir...
Posted by prabhjyotsingh <gi...@git.apache.org>.
Github user prabhjyotsingh commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/501#issuecomment-161901929
LGTM, look better than default alerts/confirm box.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---