You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by AhyoungRyu <gi...@git.apache.org> on 2017/03/06 11:30:49 UTC
[GitHub] zeppelin pull request #2100: [ZEPPELIN-2060] Make dynamic select form turn o...
GitHub user AhyoungRyu opened a pull request:
https://github.com/apache/zeppelin/pull/2100
[ZEPPELIN-2060] Make dynamic select form turn on or off using checkbox
### What is this PR for?
I added "Auto Run" checkbox for select dynamic form to make user turn on / off automatic running after the form value changed.
### What type of PR is it?
Improvement
### Todos
* [ ] - update docs after getting feedback
### What is the Jira issue?
[ZEPPELIN-2060](https://issues.apache.org/jira/browse/ZEPPELIN-2060)
### How should this be tested?
1. Apply this patch and run Zeppelin web as dev mode
```bash
# under zeppelin-web
$ yarn run dev
```
2. Go to Spark tutorial note and try to change value in select form as below screenshot imgs
### Screenshots (if appropriate)
- "Auto Run" checkbox button will be shown only in select dynamic form's dropdown menu
- _**turn on**_ "Auto Run"-> auto run right after the value changed / _**turn off**_ -> need to press `Enter`
### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? yes, maybe [this part](https://github.com/apache/zeppelin/blob/master/docs/manual/dynamicform.md#select-form)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/AhyoungRyu/zeppelin feature/turnOnOrOffAutoRun
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/2100.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 #2100
----
commit f2e32592ba3d7ba7b6073525a4d8cb908996e963
Author: AhyoungRyu <fb...@hanmail.net>
Date: 2017-03-06T11:08:31Z
Make dynamic select form turn on or off using checkbox
----
---
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] zeppelin issue #2100: [ZEPPELIN-2060] Make dynamic select form turn on or of...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/2100
@AhyoungRyu Tested and it works well.
To me, label "Auto Run" is bit hard to guess that it is related with dynamic form. Can you suggest another label which helps user better understanding?
And do you think you can add test for it?
---
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] zeppelin issue #2100: [ZEPPELIN-2060] Make dynamic select form turn on or of...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/zeppelin/pull/2100
I like this idea - could be hard to explain in a few words though :)
---
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] zeppelin pull request #2100: [ZEPPELIN-2060] Make dynamic select form turn o...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zeppelin/pull/2100
---
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] zeppelin issue #2100: [ZEPPELIN-2060] Make dynamic select form turn on or of...
Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:
https://github.com/apache/zeppelin/pull/2100
CI is green now. Ready to review again.
---
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] zeppelin issue #2100: [ZEPPELIN-2060] Make dynamic select form turn on or of...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/2100
@AhyoungRyu Sounds good!
---
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] zeppelin issue #2100: [ZEPPELIN-2060] Make dynamic select form turn on or of...
Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:
https://github.com/apache/zeppelin/pull/2100
@Leemoonsoo @felixcheung Sorry for my late update. To provide more intuitive label, I changed it from `Auto run` -> `Run on selection change` and added tooltip msg to let user know they still can run the paragraph by pressing enter even if they uncheck the checkbox.
<img width="349" alt="screen shot 2017-03-14 at 10 52 22 pm" src="https://cloud.githubusercontent.com/assets/10060731/23903934/9382ac6a-0909-11e7-86b6-6a0b904c8202.png">
If it's okay, then I'll add the test and update the docs.
---
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] zeppelin issue #2100: [ZEPPELIN-2060] Make dynamic select form turn on or of...
Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:
https://github.com/apache/zeppelin/pull/2100
CI is also green now :D
---
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] zeppelin issue #2100: [ZEPPELIN-2060] Make dynamic select form turn on or of...
Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:
https://github.com/apache/zeppelin/pull/2100
@Leemoonsoo I made `runOnSelectionChange` can be persisted under `note.json`'s `config` field like below.
<img width="456" alt="screen shot 2017-03-16 at 12 56 14 pm" src="https://cloud.githubusercontent.com/assets/10060731/23981188/fe8a5fbe-0a47-11e7-8516-ca0be81ed4dd.png">
If user tries to `check`/`uncheck` the option in dropdown menu, this value will be updated. So reloading / Zeppelin restart won't be affect to this value change.
What do you think?
And regarding
>Do you mind improve unittest to not only verify button is checked, but also verify actual dynamic form behavior, in this PR or later in the other PR?
If it's okay, I'll add the test case for this changed behaviour in another PR!
---
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] zeppelin issue #2100: [ZEPPELIN-2060] Make dynamic select form turn on or of...
Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:
https://github.com/apache/zeppelin/pull/2100
@Leemoonsoo Didn't realize that. Thanks for letting me know. Let me update again.
---
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] zeppelin issue #2100: [ZEPPELIN-2060] Make dynamic select form turn on or of...
Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:
https://github.com/apache/zeppelin/pull/2100
@Leemoonsoo I added two test cases as you said in this commit: 66c6c21d1bb435f5ade327a8f68cf3e8897e1fb3
- Check "Run on selection change" menu appears or not, when select form is created
- Check `isAutoRunTrue` value has been changed to `false` or not, if "Run on selection change" is unchecked
And also added below content to [this part](https://zeppelin.apache.org/docs/0.8.0-SNAPSHOT/manual/dynamicform.html#select-form) in `docs/manual/dynamicform.md`.
---
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] zeppelin issue #2100: [ZEPPELIN-2060] Make dynamic select form turn on or of...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/2100
Ah i realized that "Run on selection change" is not persisted, so refreshing browser resets the option. @AhyoungRyu Could you take care?
---
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] zeppelin issue #2100: [ZEPPELIN-2060] Make dynamic select form turn on or of...
Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:
https://github.com/apache/zeppelin/pull/2100
CI failed when it runs `yarn run test` and it seems relevant with this change. Let me update again.
---
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] zeppelin issue #2100: [ZEPPELIN-2060] Make dynamic select form turn on or of...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/2100
Tested and 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] zeppelin issue #2100: [ZEPPELIN-2060] Make dynamic select form turn on or of...
Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:
https://github.com/apache/zeppelin/pull/2100
@Leemoonsoo @felixcheung I think this way will be better than #2072. Can I ask review this PR?
---
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] zeppelin pull request #2100: [ZEPPELIN-2060] Make dynamic select form turn o...
Posted by AhyoungRyu <gi...@git.apache.org>.
GitHub user AhyoungRyu reopened a pull request:
https://github.com/apache/zeppelin/pull/2100
[ZEPPELIN-2060] Make dynamic select form turn on or off using checkbox
### What is this PR for?
I added "Auto Run" checkbox for select dynamic form to make user turn on / off automatic running after the form value changed.
### What type of PR is it?
Improvement
### Todos
* [ ] - update docs after getting feedback
### What is the Jira issue?
[ZEPPELIN-2060](https://issues.apache.org/jira/browse/ZEPPELIN-2060)
### How should this be tested?
1. Apply this patch and run Zeppelin web as dev mode
```bash
# under zeppelin-web
$ yarn run dev
```
2. Go to Spark tutorial note and try to change value in select form as below screenshot imgs
### Screenshots (if appropriate)
- "Auto Run" checkbox button will be shown only in select dynamic form's dropdown menu
- _**turn on**_ "Auto Run"-> auto run right after the value changed / _**turn off**_ -> need to press `Enter`
### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? yes, maybe [this part](https://github.com/apache/zeppelin/blob/master/docs/manual/dynamicform.md#select-form)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/AhyoungRyu/zeppelin feature/turnOnOrOffAutoRun
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/2100.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 #2100
----
commit f2e32592ba3d7ba7b6073525a4d8cb908996e963
Author: AhyoungRyu <fb...@hanmail.net>
Date: 2017-03-06T11:08:31Z
Make dynamic select form turn on or off using checkbox
commit 507571199f8a545022c7500c0980bbdd90208bc0
Author: AhyoungRyu <fb...@hanmail.net>
Date: 2017-03-06T13:10:14Z
Add a space next to checkbox
commit 8a0ca30fd7340c621ed51a5f1a90782d65b130eb
Author: AhyoungRyu <fb...@hanmail.net>
Date: 2017-03-14T13:53:06Z
Change label to 'Run on selection change'
commit 66c6c21d1bb435f5ade327a8f68cf3e8897e1fb3
Author: AhyoungRyu <fb...@hanmail.net>
Date: 2017-03-15T06:43:23Z
Add test case for behaviour of 'Run on selection change'
commit 4904f38659453242e06bbaedab38b3c4427f20a4
Author: AhyoungRyu <fb...@hanmail.net>
Date: 2017-03-15T07:06:07Z
Update dynamicForm docs
commit 47dcb5f64a4079ea26c7a39f5ae3955a7a710d16
Author: AhyoungRyu <fb...@hanmail.net>
Date: 2017-03-16T03:52:20Z
Persist 'runOnSelectionChange' under note.json's config field
commit 40435794f8e64244dd602e1fbd986ffcf26a9400
Author: AhyoungRyu <fb...@hanmail.net>
Date: 2017-03-16T09:12:26Z
Add 'setting: {forms: {}}' info to paragraphMock
----
---
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] zeppelin issue #2100: [ZEPPELIN-2060] Make dynamic select form turn on or of...
Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:
https://github.com/apache/zeppelin/pull/2100
@Leemoonsoo Thanks for review!
Regarding the test, sure. i'll add the test to [ParagraphActionsIT.java](https://github.com/apache/zeppelin/blob/master/zeppelin-server/src/test/java/org/apache/zeppelin/integration/ParagraphActionsIT.java).
And let me think about more about intuitive label for this. I think we also need to let them know even if they uncheck this option, still can run the dynamic form by pressing `Enter`. Anyway I will update again :)
---
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] zeppelin issue #2100: [ZEPPELIN-2060] Make dynamic select form turn on or of...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/2100
LGTM.
@AhyoungRyu Do you mind improve unittest to not only verify button is checked, but also verify actual dynamic form behavior, in this PR or later in the other PR.
---
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] zeppelin issue #2100: [ZEPPELIN-2060] Make dynamic select form turn on or of...
Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:
https://github.com/apache/zeppelin/pull/2100
@Leemoonsoo Thanks for reveiw.
Merge into branch-0.7 and master if there are no more comments
---
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] zeppelin pull request #2100: [ZEPPELIN-2060] Make dynamic select form turn o...
Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu closed the pull request at:
https://github.com/apache/zeppelin/pull/2100
---
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.
---