[ZEPPELIN-1404] invalid html structure for bootstrap in interpreter setting page by cloverhearts · Pull Request #1395 · apache/zeppelin

cloverhearts · 2016-09-02T07:23:46Z

What is this PR for?

invalid html structure for bootstrap in interpreter setting page.
for example :
correct

<div class="row">
<div class="col-md-12">
</div>
</div>

invalid now.

<div class="row">
<div class="col-md-12">
</div>
</div>
<div class="col-md-12">
</div>

There occurs a problem with the current designers, and shape.

Please, check to Screenshots in this pr.

What type of PR is it?

Bug Fix

Todos

Fixed html structure
modification to margin for checkbox.

What is the Jira issue?

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

How should this be tested?

click to interpreter menu in web ui.

Screenshots (if appropriate)

before wide screen.

after wide screen.

active stripe table, and fixed margin, and other layout.

before mobile size screen

after mobile size screen

Questions:

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

AhyoungRyu · 2016-09-05T08:18:07Z

Tested and it definitely looks better!
It's my personal preference, can we put more space between "Option" and the dropdown menu? It would be better "Options" and the dropdown menu has same gap with "Properties" and the below property table. I mean

cloverhearts · 2016-09-05T09:07:24Z

@AhyoungRyu Thank you for your feedback.
i'll fixed it

corneadoug · 2016-09-06T01:43:36Z

zeppelin-web/src/app/interpreter/interpreter.html

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


Can you tell more about that change?

@corneadoug
I attach a link, part of the note.
https://github.com/apache/zeppelin/pull/1395/files#diff-52843fa71a36ded9162a8bbb812459bcR295

I do not know the correct action on the part of the fact.
However, this part will be described as Invalid value in my IDE.

e-form will resolve this issue.

e-form is used only for the directive editable-text.
Same for the e-msd-elastic below I guess.

There is a bit of disparity between the inputs in general, it could probably be reworked separately

Thank you for answer.
I will revert to this code and create a new pr.

cloverhearts · 2016-09-06T02:33:58Z

fixed.
cd6db05

<table class="table table-striped" style="margin-top:0px;">

corneadoug · 2016-09-06T02:57:58Z

@cloverhearts actually there is also the space of the dependencies section.

It would be better to simply modify the table-striped class. at this line

I checked and that class is used only in a few cases (Credential, Configuration, Interpreter), and removing that space is making them look better)

Except for that LGTM

cloverhearts · 2016-09-06T04:44:16Z

@corneadoug
Okay, i has fixed it.

removed style table style="margin-top: 0px"
remove margin-top css in paragraph.css

c725381

corneadoug · 2016-09-06T04:57:13Z

LGTM

corneadoug · 2016-09-08T05:06:08Z

@cloverhearts Could you rebase this branch?
It should make the CI green

cloverhearts · 2016-09-08T06:04:57Z

@corneadoug
rebase is done.
ci is green!
Thank you.

corneadoug · 2016-09-08T06:08:04Z

Merging if there is no more discussions

### What is this PR for? #1395 changed alignment of html tag and it caused to make space between interpreter group and name. ### What type of PR is it? Bug Fix ### Screenshots (if appropriate) **Before** <img width="436" alt="screen shot 2016-09-22 at 11 31 35 am" src="https://cloud.githubusercontent.com/assets/8503346/18743269/7fa34b16-80b8-11e6-979a-194e7f6bef8f.png"> **After** <img width="411" alt="screen shot 2016-09-22 at 11 32 02 am" src="https://cloud.githubusercontent.com/assets/8503346/18743271/83807038-80b8-11e6-80b1-775c3369940e.png"> ### Questions: * Does the licenses files need update? no * Is there breaking changes for older versions? no * Does this needs documentation? no Author: Mina Lee <[email protected]> Closes #1448 from minahlee/showInterpreterList and squashes the following commits: 5e2315a [Mina Lee] Remove space in available interpreters list

### What is this PR for? apache#1395 changed alignment of html tag and it caused to make space between interpreter group and name. ### What type of PR is it? Bug Fix ### Screenshots (if appropriate) **Before** <img width="436" alt="screen shot 2016-09-22 at 11 31 35 am" src="https://cloud.githubusercontent.com/assets/8503346/18743269/7fa34b16-80b8-11e6-979a-194e7f6bef8f.png"> **After** <img width="411" alt="screen shot 2016-09-22 at 11 32 02 am" src="https://cloud.githubusercontent.com/assets/8503346/18743271/83807038-80b8-11e6-80b1-775c3369940e.png"> ### Questions: * Does the licenses files need update? no * Is there breaking changes for older versions? no * Does this needs documentation? no Author: Mina Lee <[email protected]> Closes apache#1448 from minahlee/showInterpreterList and squashes the following commits: 5e2315a [Mina Lee] Remove space in available interpreters list

fixed to correct html(bootstrap) syntax for interpreter setting page.

771c901

corneadoug reviewed Sep 6, 2016
View reviewed changes

cloverhearts added 2 commits September 6, 2016 11:26

revert to e-form

5c6e476

Property between Table - removed margin

cd6db05

revert - style margin-top-0 and removed margin-top in paragraph.css

c725381

Merge branch 'master' into dev/interpreter-tag-fixed

305664c

asfgit closed this in 9bce03c Sep 8, 2016

AhyoungRyu mentioned this pull request Sep 17, 2016

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

Closed

minahlee mentioned this pull request Sep 22, 2016

[MINOR] Remove spaces between intepreter group and name #1448

Closed

Conversation

cloverhearts commented Sep 2, 2016

What is this PR for?

What type of PR is it?

Todos

What is the Jira issue?

How should this be tested?

Screenshots (if appropriate)

before wide screen.

after wide screen.

before mobile size screen

after mobile size screen

Questions:

Uh oh!

AhyoungRyu commented Sep 5, 2016

Uh oh!

cloverhearts commented Sep 5, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

corneadoug Sep 6, 2016

Choose a reason for hiding this comment

Uh oh!

cloverhearts Sep 6, 2016

Choose a reason for hiding this comment

Uh oh!

corneadoug Sep 6, 2016

Choose a reason for hiding this comment

Uh oh!

cloverhearts Sep 6, 2016

Choose a reason for hiding this comment

Uh oh!

cloverhearts commented Sep 6, 2016

Uh oh!

corneadoug commented Sep 6, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cloverhearts commented Sep 6, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

corneadoug commented Sep 6, 2016

Uh oh!

corneadoug commented Sep 8, 2016

Uh oh!

cloverhearts commented Sep 8, 2016

Uh oh!

corneadoug commented Sep 8, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cloverhearts commented Sep 5, 2016 •

edited

Loading

corneadoug commented Sep 6, 2016 •

edited

Loading

cloverhearts commented Sep 6, 2016 •

edited

Loading