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-1404] invalid html structure for bootstrap in interpreter setting page #1395

Closed
wants to merge 5 commits into from
Closed

[ZEPPELIN-1404] invalid html structure for bootstrap in interpreter setting page #1395

wants to merge 5 commits into from

Conversation

cloverhearts
Copy link
Member

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.

beforewide

after wide screen.

afterwide

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

before mobile size screen

beforemobile

after mobile size screen

aftermobile

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

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
Copy link
Member Author

cloverhearts commented Sep 5, 2016

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

@@ -286,7 +299,7 @@ <h3 class="interpreter-title">{{setting.name}}
<td>
<textarea ng-if="valueform.$visible" ng-model="dep.exclusions"
placeholder="(Optional) comma separated groupId:artifactId list"
form="valueform"
e-form="valueform"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you tell more about that change?

Copy link
Member Author

Choose a reason for hiding this comment

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

@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.
2016-09-06 10 52 13
e-form will resolve this issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

@cloverhearts
Copy link
Member Author

fixed.
cd6db05

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

2016-09-06 11 31 00

@corneadoug
Copy link
Contributor

corneadoug commented Sep 6, 2016

@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
Copy link
Member Author

cloverhearts commented Sep 6, 2016

@corneadoug
Okay, i has fixed it.

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

c725381

result

@corneadoug
Copy link
Contributor

LGTM

@corneadoug
Copy link
Contributor

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

@cloverhearts
Copy link
Member Author

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

@corneadoug
Copy link
Contributor

Merging if there is no more discussions

@asfgit asfgit closed this in 9bce03c Sep 8, 2016
asfgit pushed a commit that referenced this pull request Sep 25, 2016
### 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 <minalee@apache.org>

Closes #1448 from minahlee/showInterpreterList and squashes the following commits:

5e2315a [Mina Lee] Remove space in available interpreters list
pedrozatta pushed a commit to pedrozatta/zeppelin that referenced this pull request Oct 27, 2016
### 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 <minalee@apache.org>

Closes apache#1448 from minahlee/showInterpreterList and squashes the following commits:

5e2315a [Mina Lee] Remove space in available interpreters list
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants