Skip to content

Conversation

@AhyoungRyu
Copy link
Contributor

@AhyoungRyu AhyoungRyu commented Sep 18, 2016

What is this PR for?

"Currently there are no properties and dependencies set for this interpreter" message is shown when there are no properties & dependencies in interpreter setting page. But this message can be seen even in the edit mode. It's quite wired. This message needs to be shown only in non-editable mode.

The edit, restart and remove button are same.

  • edit button doesn't do any work after changing to edit mode
  • restart button: it doesn't make sense that someone is editing and restarting interpreter at the same time
  • remove button: Maybe someone wants to remove the interpreter while he is editing the interpreter. But it would be better this button is shown only in non-editable mode for the consistency with the other buttons.

For the above reasons, I changed those buttons to be shown in non-editable mode only. Save & Cancel buttons are enough to the edit page I think. Please see the attached gif image.

What type of PR is it?

Bug Fix

What is the Jira issue?

How should this be tested?

  1. After applying this patch and build only zeppelin-web with ./grunt build under zeppelin-web.
  2. You can quickly check this change in dev mode with ./grunt serve.
  3. Go to Interpreter menu and click "edit" button on Angular interpreter to see "Currently there are no properties and dependencies set for this interpreter" message. Can also check the existence of edit, restart and remove buttons.

Screenshots (if appropriate)

  • Before
    before
  • After
    after

Questions:

  • Does the licenses files need update? no
  • Is there breaking changes for older versions? no
  • Does this needs documentation? no

<textarea ng-if="valueform.$visible" ng-model="dep.exclusions"
placeholder="(Optional) comma separated groupId:artifactId list"
form="valueform"
e-form="valueform"
Copy link
Contributor

@corneadoug corneadoug Sep 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No e-form is needed here,
Actually the e-msd-elastic below should also be msd-elastic
those e-properties are only necessary in elements like editable-text

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@corneadoug Ah didn't know that. I'm using IntelliJ now, it said like below for form="valueform".
screen shot 2016-09-19 at 4 38 50 pm

Anyway I reverted it :)

@corneadoug
Copy link
Contributor

tested, except for the comment, LGTM

@corneadoug
Copy link
Contributor

@AhyoungRyu Test, Nice Improvements
Can you just rebase to have a green CI?
After that, Merging if there is no more discussions

@asfgit asfgit closed this in f2a5c59 Sep 22, 2016
pedrozatta pushed a commit to pedrozatta/zeppelin that referenced this pull request Oct 27, 2016
…tart" and "remove" btn in edit mode

### What is this PR for?
"Currently there are no properties and dependencies set for this interpreter" message is shown when there are no properties & dependencies in interpreter setting page. But this message can be seen even in the edit mode. It's quite wired. This message needs to be shown only in non-editable mode.

The `edit`, `restart` and `remove` button are same.
 - `edit` button doesn't do any work after changing to edit mode
 - `restart` button: it doesn't make sense that someone is editing and restarting interpreter at the same time
 - `remove` button: Maybe someone wants to remove the interpreter while he is editing the interpreter. But it would be better this button is shown only in non-editable mode for the consistency with the other buttons.

For the above reasons, I changed those buttons to be shown in non-editable mode only. `Save` & `Cancel` buttons are enough to the edit page I think. Please see the attached gif image.

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

### What is the Jira issue?

### How should this be tested?
1. After applying this patch and build only `zeppelin-web` with `./grunt build` under `zeppelin-web`.
2. You can quickly check this change in dev mode with `./grunt serve`.
3. Go to `Interpreter` menu and click "edit" button on Angular interpreter to see "Currently there are no properties and dependencies set for this interpreter" message. Can also check the existence of `edit`, `restart` and `remove` buttons.

### Screenshots (if appropriate)
 - Before
![before](https://cloud.githubusercontent.com/assets/10060731/18616987/79b40d3a-7e01-11e6-85a1-e3888ef7f2f5.gif)

 - After
![after](https://cloud.githubusercontent.com/assets/10060731/18616988/7df1d094-7e01-11e6-9146-400e36b2392a.gif)

### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no

Author: AhyoungRyu <[email protected]>
Author: AhyoungRyu <[email protected]>

Closes apache#1434 from AhyoungRyu/fix/editMode and squashes the following commits:

4433f86 [AhyoungRyu] Make same width 'properties'&'dependencies' table
2e50f5d [AhyoungRyu] Revert 'form'
0c6c937 [AhyoungRyu] Fix minor issues in interpreter setting edit mode
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