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

Submit Hotkey #33

Closed
wants to merge 6 commits into from
Closed

Submit Hotkey #33

wants to merge 6 commits into from

Conversation

NDevTK
Copy link
Contributor

@NDevTK NDevTK commented Jul 27, 2019

No description provided.

content.js Show resolved Hide resolved
@ajayyy
Copy link
Owner

ajayyy commented Jul 27, 2019

After looking at it again, I can see how it can work, but I don't really like doing it this way, I feel it's easier to make mistakes. I think it's best to find all the sponsors in the video, THEN submit. This makes it so the user is sure those are the correct sponsors.

If you do want it this way for yourself, you can make it an option, but I don't want that to be the default.

@NDevTK
Copy link
Contributor Author

NDevTK commented Jul 27, 2019

Submitting one at a time has the benefit of being able to cancel per sponsor
I suggest a different key to be used to add more times instead of submitting

@ajayyy
Copy link
Owner

ajayyy commented Jul 27, 2019

That could be resolved with a UI change.

And sadly, you can't just use a variable created there because it has to sync up with the popup. I recommend checking if the document.getElementById("submitButton").style != "none".

I'll merge it in in if you make it an option and not on by default.

@NDevTK
Copy link
Contributor Author

NDevTK commented Jul 27, 2019

I dont like the global popup as its not per video

@NDevTK NDevTK mentioned this pull request Aug 1, 2019
@NDevTK NDevTK changed the base branch from master to experimental August 1, 2019 10:19
content.js Outdated Show resolved Hide resolved
Settings.hotkeys = {}; // TEMP
Settings.onekey = false; // TEMP
Settings.hotkeys.submit = "Quote"; // TEMP
Settings.hotkeys.start = "Semicolon"; // TEMP
Copy link
Owner

Choose a reason for hiding this comment

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

Can you set this up so the settings save? Right now it does nothing as it is by default just the same

Copy link
Contributor Author

@NDevTK NDevTK Aug 3, 2019

Choose a reason for hiding this comment

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

I would but theirs no place to change them or to store the default values

Copy link
Owner

Choose a reason for hiding this comment

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

Yea, for now you can put it in the options section of the popup

Copy link
Contributor Author

@NDevTK NDevTK Aug 3, 2019

Choose a reason for hiding this comment

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

There is no option page currently???

Copy link
Owner

Choose a reason for hiding this comment

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

Yes there is, click the options button the popup

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Waiting for the UI changes

content.js Show resolved Hide resolved
content.js Outdated Show resolved Hide resolved
@ajayyy ajayyy added the WIP label Aug 10, 2019
@ajayyy ajayyy added the stuck label Aug 19, 2019
@ajayyy
Copy link
Owner

ajayyy commented Oct 31, 2019

#163

@ajayyy ajayyy closed this Oct 31, 2019
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.

2 participants