search

az0 / linkgopher

Firefox/Google Chrome add-on: Extracts all links from web page, sorts them, removes duplicates, and displays them in a new tab for inspection or copy and paste into other systems.
GNU General Public License v3.0
257 stars 56 forks source link

Patch 3 #15

Open userlinksgopher opened 6 years ago

userlinksgopher commented 6 years ago

Added filtering on text Added extraction of links from selection Added message "No links" Updated locale ru Added filteringDomains (addNodes)

userlinksgopher commented 6 years ago

Hi, please reply. Patch will be merged into the master branch?

az0 commented 6 years ago

Thank you for the code. I would like a little time to review. When the review is all done and any issues are resolved, I will merge it into the master branch.

(In general it is easier for me to review smaller chunks, such as one feature or one bug fix per pull request, but overall this looks like not a large change.)

az0 commented 6 years ago

I tested the new functionality in Firefox 57, and the first three features work well. I started a new mini-site with testing pages. For example, there is a simple page without links.

Would you please elaborate on filteringDomains?

Also, would you please make two changes?

First, it searches now by either the link URL or link text, but the user prompt is no longer accurate.

Enter a string of characters to search for within the link. Links without this string will be ignored.

How about this?

Enter a string of characters to search for within the link address or link text. Links without this string will be ignored.

This would be easy for me to change, but you would need to update the translation too.

Second, searching by selection may not be intuitive because it not stated in the user interface. If the user highlights a portion, he may be surprised not all links are returned. If the user wants only a portion of links to be returned, he may not know that highlighting enables this functionality.

Therefore, I propose to add a checkbox in popup.html called Search in selection.

The default state of the checkbox is either

  1. Off
  2. Remembered (stored in the configuration)
  3. On if and only if there is an active selection in the current tab
userlinksgopher commented 6 years ago

Would you please elaborate on filteringDomains?

I knew that this question will be! :) That's my fault... Of course, the string "Added filteringDomains (addNodes)" should be removed from commit. In fact, in the function addNodes //filteringDomains to be replaced by //domains. I left it there that would know where the string or object is sent to the function.

First, it searches now by either the link URL or link text, but the user prompt is no longer accurate.

Enter a string of characters to search for within the link. Links without this string will be ignored.

How about this?

Enter a string of characters to search for within the link address or link text. Links without this string will be ignored.

This would be easy for me to change, but you would need to update the translation too.

I agree. It can be done

Second, searching by selection may not be intuitive because it not stated in the user interface. If the user highlights a portion, he may be surprised not all links are returned. If the user wants only a portion of links to be returned, he may not know that highlighting enables this functionality.

Therefore, I propose to add a checkbox in popup.html called Search in selection.

The default state of the checkbox is either

  1. Off
  2. Remembered (stored in the configuration)
  3. On if and only if there is an active selection in the current tab

Create extra controls... Maybe change the text on the button when there is a selection? If I understand correctly, it is necessary from popup.js determine whether there is a selection. But I don't know how to do... Immediately on https://developer.mozilla.org not found. Where to find the answer? Need help, please :)