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

userscript version of the Chrome extension #21

Closed
wants to merge 5 commits into from

Conversation

odnar-dev
Copy link
Contributor

@odnar-dev odnar-dev commented Mar 12, 2021

closes #20

this is just a copy of /project/useful-forks.js
fixed : useful-forks#9
fixed : useful-forks#4
only call if GITHUB_ACCESS_TOKEN has been set up or show an error
@odnar-dev
Copy link
Contributor Author

menu to make it easy to add GITHUB_ACCESS_TOKEN or change it
1
show error if GITHUB_ACCESS_TOKEN has not been set up
2
hover color is adapted to GitHub's Night-Mode. (need better colors)
3

@payne911
Copy link
Collaborator

payne911 commented Mar 13, 2021

That is looking like some really great work: I'll review the PR in the coming days. Thanks a lot for your contribution !

Are you aware of whether it would be possible not to duplicate the JavaScript code used for the Chrome plugin ? I know refined-github somehow manages to use the same code for all their supported browsers.

Maybe @fregante 's expertise could be helpful here.

@payne911 payne911 changed the title userscript version of the Chrome extension userscript version of the Chrome extension (closes #20) Mar 14, 2021
@payne911 payne911 changed the title userscript version of the Chrome extension (closes #20) userscript version of the Chrome extension Mar 14, 2021
@payne911
Copy link
Collaborator

(Sorry, I thought changing the title would link the other issue, but I ended up having to edit your OP.)

@odnar-dev
Copy link
Contributor Author

3

@payne911
Copy link
Collaborator

payne911 commented Mar 17, 2021

document.addEventListener('pjax:end', init);

Is that what fixes #9 ?

Also, does Violentmonkey require the script to be within that (function() { /* code */ })(); wrapper ?

@odnar-dev
Copy link
Contributor Author

document.addEventListener('pjax:end', init);
Is that what fixes #9 ?

yes , github use for some pages Pjax to deliver a faster browsing experience without reloading the full page.
check this

does Violentmonkey require the script to be within that (function() { /* code */ })(); wrapper ?

i don't think so , just a habit

@payne911
Copy link
Collaborator

Would you mind creating 2 extra PRs for the fixes of #4 and #9 ? I'd do it myself, but I'll respect your "ownership" of the solutions, in case you wanted to be labeled as a contributor. I'd merge both those PRs into the Chrome script.

For this current PR, I don't have a lot of time to test it just yet. I'll eventually try to simply add your // ==UserScript== right at the top of the Chrome script.

@payne911
Copy link
Collaborator

Alright, #25 was perfect, thanks for that!

I tried to incorporate your fix for #9 but couldn't get it to work properly. Have you tested if those cases work properly:

  1. landing directly on the Forks page (test with: https://github.com/useful-forks/useful-forks.github.io/network/members)
  2. refreshing the page when on the Forks page works (just press F5 when at the URL presented above)

@payne911 payne911 mentioned this pull request Mar 19, 2021
@payne911
Copy link
Collaborator

Alright, now that I've incorporated both of your side-fixes into the main branch, it could be interesting to look into what this actually has to offer.

Have you tried incorporating your code directly into the Chrome plugin's script and seeing how well it works for people with and without Violentmonkey ?

@odnar-dev
Copy link
Contributor Author

Violentmonkey is an userscript manager
this part of the script use a group of special apis provided only by an userscript manager.

// get token from local storage provided by the userscript manager.
let GITHUB_ACCESS_TOKEN = GM_getValue('GITHUB_ACCESS_TOKEN')
// add menu 
GM_registerMenuCommand("Set Github Access Token", setPersonalToken)
GM_registerMenuCommand("Generate New Access Token", newPersonalToken);
// store token in local storage provided by the userscript manager.
function setPersonalToken(){
    var mess = "Personal Access Token";
    var caseShow = GITHUB_ACCESS_TOKEN;
    var getpersonalToken = prompt(mess, caseShow);
    GITHUB_ACCESS_TOKEN = (getpersonalToken===null? GITHUB_ACCESS_TOKEN : getpersonalToken)
    GM_setValue("GITHUB_ACCESS_TOKEN", GITHUB_ACCESS_TOKEN)
}
// open url to generate a new personal token in new tab
function newPersonalToken(){
  let tabControl = GM_openInTab("https://github.com/settings/tokens/new?scopes=repo&description=UsefulFork")
  tabControl.onclose = () => setPersonalToken();
}

this should work in most other userscript managers (like Greasemonkey, Tampermonkey..)

@payne911
Copy link
Collaborator

payne911 commented May 27, 2021

My main concern is the duplication of code. Is there a way to integrate it within the chrome script in a non-intrusive way?

I'm already bothered by a large portion of the code being duplicated, I wouldn't want to have to maintain a third duplicate.

@odnar-dev
Copy link
Contributor Author

hey any news about this ?

@Metacinnabar
Copy link

Any update? This would be a great addition!

@payne911
Copy link
Collaborator

payne911 commented Nov 11, 2021

As mentioned, I'm kind of bothered by the duplication of code. See #21 (comment).

@odnar-dev
Copy link
Contributor Author

i'm kind of bothered by the duplication of code

what about creating a separate file that contain all shared functions and variables
then simply include it to chrome script and userscript

@payne911
Copy link
Collaborator

what about creating a separate file that contain all shared functions and variables

Maybe that could work.

As a Java dev, what comes to mind is abstracting with an interface that has a few default methods for the shared ones, and then some different implementations for the Chrome and Monkey plugins.

@payne911
Copy link
Collaborator

payne911 commented Jan 19, 2023

The plugin has been heavily modified (see 63b9a3c).

I'm closing the PR because it would basically need a rewrite anyways.

@payne911 payne911 closed this Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a userscript version of the Chrome extension
3 participants