Skip to content

[ZEPPELIN-687] Display paragraph id in config dropdown menu - #739

Closed
doanduyhai wants to merge 1 commit into
apache:masterfrom
doanduyhai:ZEPPELIN-687
Closed

[ZEPPELIN-687] Display paragraph id in config dropdown menu#739
doanduyhai wants to merge 1 commit into
apache:masterfrom
doanduyhai:ZEPPELIN-687

Conversation

@doanduyhai

Copy link
Copy Markdown
Contributor

What is this PR for?

Display paragraph id in the config drop down menu

This is a sub-task of epic ZEPPELIN-635

What type of PR is it?

[Improvement]

Todos

  • - Trivial code review & test

Is there a relevant Jira issue?

ZEPPELIN-687

How should this be tested?

  1. git fetch origin pull/739/head:DisplayParagraphId
  2. git checkout DisplayParagraphId
  3. mvn clean package -DskipTests
  4. bin/zeppelin-daemon.sh restart
  5. Create any note
  6. Click on the config drop down menu of any paragraph to see the paragraph id

Screenshots (if appropriate)

image

Questions:

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

@Leemoonsoo

Copy link
Copy Markdown
Member

Looks good to me

@corneadoug

Copy link
Copy Markdown
Contributor

How about presenting it as the title:

<li ng-click="$event.stopPropagation()" style="text-align:center;margin-top:4px;">
{{paragraph.id}}
</li>
<li role="separator" class="divider"></li>

screen shot 2016-02-22 at 9 31 42 am

@doanduyhai

Copy link
Copy Markdown
ContributorAuthor

@corneadoug Good idea, but why do we need the ng-click="$event.stopPropagation()" ?

@corneadoug

Copy link
Copy Markdown
Contributor

The drop-down menu is closing on mouse-up event.
Since we want to select the paragraph id to do a copy/paste, its better to remove that behavior on this element.

@doanduyhai

Copy link
Copy Markdown
ContributorAuthor

Ok, makes sense, thanks, I'll update the PR accordingly

@doanduyhai

Copy link
Copy Markdown
ContributorAuthor

Done, see updated screenshot

@felixcheung

Copy link
Copy Markdown
Member

Is it feasible to have a copy button? or is it to crowded?
http://fortawesome.github.io/Font-Awesome/icon/clipboard/

@doanduyhai

Copy link
Copy Markdown
ContributorAuthor

I've tried to add an icon before and it overflows the maximum width of the popup. We can also reduce the font-size so the long paragraph id will fit in but it looks ugly

@corneadoug

Copy link
Copy Markdown
Contributor

LGTM

@Leemoonsoo

Copy link
Copy Markdown
Member

LGTM and merge if there're no more discussions

Sign up for freeto 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.

4 participants

@doanduyhai@Leemoonsoo@corneadoug@felixcheung