Skip to content

[ZEPPELIN-2672] Fix interpreter repos url tooltip#2423

Closed
necosta wants to merge 4 commits intoapache:masterfrom
nokia:zeppelin2672
Closed

[ZEPPELIN-2672] Fix interpreter repos url tooltip#2423
necosta wants to merge 4 commits intoapache:masterfrom
nokia:zeppelin2672

Conversation

@necosta
Copy link
Contributor

@necosta necosta commented Jun 21, 2017

What is this PR for?

Fixing regression on showing interpreter repos tooltip

What type of PR is it?

Bug Fix

What is the Jira issue?

How should this be tested?

Zeppelin > Interpreter > Repository > (Mouse over) central/local/other

Screenshots

After this fix:
image

Questions:

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

@necosta necosta closed this Jun 21, 2017
@necosta necosta reopened this Jun 21, 2017
<ul class="noDot">
<li class="liVertical" ng-repeat="repo in repositories">
<a tabindex="0" class="btn btn-info" role="button"
popover-trigger="focus"
Copy link
Member

Choose a reason for hiding this comment

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

Can we replace all popover angualr directives?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. There was just one more. Fixed in latest commit.

Copy link
Member

Choose a reason for hiding this comment

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

Then I think we can remove the popover directive related files as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

Choose a reason for hiding this comment

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

Checked tooltip now works well. +1
But the tooltip for the first repo button is trimmed. Can we set tooltip position for repo buttons?

image

@necosta necosta closed this Jun 21, 2017
@necosta necosta reopened this Jun 21, 2017
@necosta necosta closed this Jun 22, 2017
@necosta necosta reopened this Jun 22, 2017
@necosta necosta closed this Jun 23, 2017
@necosta necosta reopened this Jun 23, 2017
style="color: white;"
popover-trigger="focus"
popover-html-unsafe="{{prop.tooltip}}"></a>
uib-tooltip="{{prop.tooltip}}"></a>
Copy link
Member

Choose a reason for hiding this comment

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

I think we need to use uib-tooltip-html

image

@1ambda
Copy link
Member

1ambda commented Jun 24, 2017

Checked that works well. LGTM except for 2 comments i left.

@1ambda
Copy link
Member

1ambda commented Jun 26, 2017

ping @necosta

@necosta
Copy link
Contributor Author

necosta commented Jun 26, 2017

Yes, @1ambda , expect updates later today. Have a few challenges on the column selector script...

@necosta
Copy link
Contributor Author

necosta commented Jun 27, 2017

@1ambda , ready for review. Thanks!

<li>Zeppelin consider values as discrete when the values contain string value
or the number of distinct values are bigger than 5% of total number of values.</li>
<li>Size field button turns to grey when the option you chose is not valid.</li>`
tooltip: `This option is only valid for numeric fields.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To me, it looked this message could be improved. Any suggestions, please let me know. I'm not sure my new sentence is clear as well :)

Copy link
Member

Choose a reason for hiding this comment

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

Looks good to me. It's more concise.

@necosta necosta closed this Jun 27, 2017
@necosta necosta reopened this Jun 27, 2017
or the number of distinct values are bigger than 5% of total number of values.</li>
<li>Size field button turns to grey when the option you chose is not valid.</li>`
tooltip: `This option is only valid for numeric fields.
When data in each axis is discrete,
Copy link
Member

Choose a reason for hiding this comment

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

This too.

Copy link
Member

@1ambda 1ambda left a comment

Choose a reason for hiding this comment

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

LGTM. merge if no more discussion

@asfgit asfgit closed this in c59f77d Jun 29, 2017
@liuhengzhi liuhengzhi deleted the zeppelin2672 branch July 3, 2018 09:14
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

Comments