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

Feature Request: "Link to" document should have copy link to clipboard #8263

Closed
styfle opened this issue Sep 13, 2016 · 19 comments
Closed

Feature Request: "Link to" document should have copy link to clipboard #8263

styfle opened this issue Sep 13, 2016 · 19 comments
Labels
Feature:Discover Discover Application good first issue low hanging fruit release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure

Comments

@styfle
Copy link

styfle commented Sep 13, 2016

Describe the feature: Copy link to clipboard for a document
The "Link to" feature is really cool because it links directly to the document.
The way our team likes to use it is to copy the link to clipboard and paste in an email or slack chat.

It appears there is already a copy link to clipboard for the query so could this feature also be added to the document?

image

@Bargs
Copy link
Contributor

Bargs commented Sep 13, 2016

I'm not totally against it, but since it's already a link can't you just right click and "Copy Link Address" (or your OS/Browser equivalent)?

screen shot 2016-09-13 at 5 36 02 pm

@styfle
Copy link
Author

styfle commented Sep 13, 2016

@Bargs Yes that is what we do now, however training non-web developers is difficult since the wording is different per browser and it takes additional clicks.

I sold our support team on Kibana/Elasticsearch by saying "it's google search for our error log".

Browsers have a proper copy to clipboard implementation and I don't think there is a lot of overhead to add this feature since the logic is already there, all we need is a button in the UI.

@Bargs
Copy link
Contributor

Bargs commented Sep 13, 2016

Makes sense, thanks for the additional explanation!

@styfle
Copy link
Author

styfle commented Sep 13, 2016

How should this look? I made a mockup what I think this could look like.

Mockup

image

This includes the URL shortener but I'm not entirely sure how the shortener works so maybe it won't apply in this case.

HTML

<div class="form-group">
    <share-object-url share-as-embed="false" class="ng-isolate-scope">
        <div class="input-group">
            <a class="" ng-href="#/doc/legacy-error-*/legacy-error-2016.09.13/Error/?id=AVcldYBUfRELoe1L0lQ_" href="#/doc/legacy-error-*/legacy-error-2016.09.13/Error/?id=AVcldYBUfRELoe1L0lQ_" style="width: 100%;">
                <small class="ng-binding" style="font-family:monospace;">Link to /legacy-error-2016.09.13/Error/AVcldYBUfRELoe1L0lQ_</small>
            </a>
            <button class="shorten-button" tooltip="Generate Short URL" ng-click="generateShortUrl()" ng-disabled="shortGenerated">
                <span aria-hidden="true" class="fa fa-compress"></span>
            </button>
            <button class="clipboard-button" tooltip="Copy to Clipboard" ng-click="copyToClipboard()">
                <span aria-hidden="true" class="fa fa-clipboard"></span>
            </button>
        </div>
    </share-object-url>
</div>

@Bargs
Copy link
Contributor

Bargs commented Sep 13, 2016

The URL shortener is probably unnecessary here, it's mostly useful for shortening long dashboard and visualization URLs that are too long for IE. That shouldn't be a problem for these simple doc links.

We try to save as much vertical space wherever we can, so I would keep the link inline with the tabs and just put the copy button on the far right of the same line.

@styfle
Copy link
Author

styfle commented Sep 14, 2016

Those are 2 good points. I made a couple more mockups below.

Mockup 2

image

HTML 2

<td colspan="3">
    <a class="pull-right" ng-href="#/doc/legacy-error-*/legacy-error-2016.09.13/Error/?id=AVcldXFetAiHB030HE_K" href="#/doc/legacy-error-*/legacy-error-2016.09.13/Error/?id=AVcldXFetAiHB030HE_K">
        <small class="ng-binding">Link to /legacy-error-2016.09.13/Error/AVcldXFetAiHB030HE_K</small>
    </a>
    <span aria-hidden="true" class="fa fa-clipboard pull-right" style="margin-right: 5px;"></span>
    <doc-viewer><!-- doc viewer omitted for brevity --></doc-viewer>
</td>

Mockup 3

image

HTML 3

<td colspan="3">
    <span aria-hidden="true" class="fa fa-clipboard pull-right" style="margin-right: 5px;"></span>
    <a class="pull-right" ng-href="#/doc/legacy-error-*/legacy-error-2016.09.13/Error/?id=AVcldXFetAiHB030HE_K" href="#/doc/legacy-error-*/legacy-error-2016.09.13/Error/?id=AVcldXFetAiHB030HE_K">
        <small class="ng-binding">Link to /legacy-error-2016.09.13/Error/AVcldXFetAiHB030HE_K</small>
    </a>
    <doc-viewer><!-- doc viewer omitted for brevity --></doc-viewer>
</td>

How does this look?

@Bargs
Copy link
Contributor

Bargs commented Sep 14, 2016

I like the one on the left, but the one on the right is more consistent with our other copy buttons :)

@alt74 what do you think?

@alt74
Copy link

alt74 commented Sep 14, 2016

@Bargs I like how GitHub puts the URL into an entry field. because that way you can keep the width of the box under control. it doesn't really matter how long the URL actually is - just click into the field, select all to copy. voila. align copy button on the right with entry field on the left.

@Bargs
Copy link
Contributor

Bargs commented Sep 14, 2016

@styfle: A couple notes. @alt74 and I chatted a bit and realized displaying the full path to the doc is kinda silly since the link is meant to be clickable. Instead, let's just replace it with the word "Link".

It was also pointed out to me that in recent updates to our "Share" UI, we no longer use icons for the "copy" button, instead it's just text:

screen shot 2016-09-14 at 6 02 16 pm

So, putting the two together, let's create two textual links, floating to the right, inline with the tabs, called "Link" and "Copy". Make sense? Sorry for the complications :)

@styfle
Copy link
Author

styfle commented Sep 15, 2016

@Bargs @alt74 How about "View" and "Copy" since those are both action words?

@Bargs
Copy link
Contributor

Bargs commented Sep 15, 2016

@styfle "View" sounds good to me! I didn't particularly like Link but I couldn't come up with anything better.

@styfle
Copy link
Author

styfle commented Sep 15, 2016

@Bargs Okay do you want me to submit a PR?

@Bargs
Copy link
Contributor

Bargs commented Sep 15, 2016

If you're willing to tackle it, that would be great!

@styfle
Copy link
Author

styfle commented Sep 16, 2016

My angular skills are weak, but I will try to get a PR today.

@styfle
Copy link
Author

styfle commented Sep 16, 2016

@Bargs I'm almost there. For some reason, the click event is not firing. Can you take a look at this commit on the copy-doc-link branch I made?

@Bargs
Copy link
Contributor

Bargs commented Sep 16, 2016

Only had time to take a quick glance, but does a simple clicker handler work? Like alert('foo');? If so, my guess is that the share directive is not in scope in that template.

@styfle
Copy link
Author

styfle commented Sep 19, 2016

@Bargs I tried alert('foo') and it didn't work.

<td colspan="{{ columns.length + 2 }}">
  <div class="pull-right">
    <a ng-href="#/doc/{{indexPattern.id}}/{{row._index}}/{{row._type}}/?id={{row._id | uriescape}}">
      <small>View</small>
    </a>
    <span style="display:inline-block; width:10px"><!--spacer--></span>
    <a ng-click="alert('foo')">
      <small>Copy</small>
    </a>
  </div>
  <doc-viewer hit="row" filter="filter" columns="columns" index-pattern="indexPattern"></doc-viewer>
</td>

Any other ideas?

@Bargs
Copy link
Contributor

Bargs commented Sep 20, 2016

@styfle could you put up a PR? Then I can pull down your changes and take a look.

@timroes timroes added Feature:Discover Discover Application Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Sep 16, 2018
@lukasolson
Copy link
Member

This feature request is an interesting idea but since its opening we have not seen enough feedback that it is a feature we should pursue. We prefer to close this issue as a clear indication that we are not going to work on this at this time. We are always open to reconsidering this in the future based on compelling feedback; despite this issue being closed please feel free to leave feedback on the proposal (including +1s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Discover Discover Application good first issue low hanging fruit release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants