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

[ZEPPELIN-1446] Fix broken layout of Create new interpreter UI. #1431

Closed
wants to merge 5 commits into from

Conversation

astroshim
Copy link
Contributor

What is this PR for?

This PR fixes broken layout of Create new interpreter UI.

What type of PR is it?

Bug Fix

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-1446

How should this be tested?

  • Try to create new interpreter on the Interpreters menu.

Screenshots (if appropriate)

  • before
    image
  • after
    image

Questions:

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

@AhyoungRyu
Copy link
Contributor

@astroshim Good catch and looks better!
Can we make "A" and "B" have same space? Need more space to "A" :)
screenshot

@astroshim
Copy link
Contributor Author

@AhyoungRyu Thank you for your kind review. I just fix the space.

@corneadoug
Copy link
Contributor

@astroshim

Could we maybe try to keep the same spacing style as the interpreter edit?

Interpreter Edit:

screen shot 2016-09-19 at 3 21 26 pm

Interpreter Create:

screen shot 2016-09-19 at 3 20 40 pm

@AhyoungRyu
Copy link
Contributor

@corneadoug Actually I did that work in here... :)

@corneadoug
Copy link
Contributor

@AhyoungRyu I was talking about the spacing below Option, Properties and Dependencies.
It stayed the same in your PR

@astroshim
Copy link
Contributor Author

Thank you @AhyoungRyu and @corneadoug .
I just fixed space problems.

@AhyoungRyu
Copy link
Contributor

@corneadoug Ah I got it. I'll do the same work as @astroshim did in my PR then :D
@astroshim Thanks for your quick response!

@corneadoug
Copy link
Contributor

Tested LGTM
@astroshim Can you just rebase? That should make the CI green

@astroshim
Copy link
Contributor Author

I rebased but build failed.

@astroshim
Copy link
Contributor Author

re-build CI

@astroshim astroshim closed this Sep 22, 2016
@astroshim astroshim reopened this Sep 22, 2016
@astroshim
Copy link
Contributor Author

re-build CI

@astroshim astroshim closed this Sep 22, 2016
@astroshim astroshim reopened this Sep 22, 2016
@AhyoungRyu
Copy link
Contributor

CI is green now! LGTM 👍

@corneadoug
Copy link
Contributor

Merging if there is no more discussions

@asfgit asfgit closed this in 2cb39f9 Sep 23, 2016
pedrozatta pushed a commit to pedrozatta/zeppelin that referenced this pull request Oct 27, 2016
### What is this PR for?
This PR fixes broken layout of `Create new interpreter` UI.

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

### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-1446

### How should this be tested?
- Try to create new interpreter on the Interpreters menu.

### Screenshots (if appropriate)
- before
![image](https://cloud.githubusercontent.com/assets/3348133/18592384/b3bfe120-7c71-11e6-80c2-31d0b4363009.png)

- after
![image](https://cloud.githubusercontent.com/assets/3348133/18592410/c777aef0-7c71-11e6-9379-9b424823fbed.png)

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

Author: astroshim <hsshim@nflabs.com>

Closes apache#1431 from astroshim/ZEPPELIN-1446 and squashes the following commits:

729215b [astroshim] Merge branch 'master' into ZEPPELIN-1446
69cc1a0 [astroshim] Merge branch 'master' into ZEPPELIN-1446
fada36b [astroshim] fix spaces
2d3ec76 [astroshim] fix space
5f0a461 [astroshim] fix align checkboxs.
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.

3 participants