Skip to content

Add shortcut#10

Open
simepy wants to merge 7 commits intorenanbr:masterfrom
simepy:master
Open

Add shortcut#10
simepy wants to merge 7 commits intorenanbr:masterfrom
simepy:master

Conversation

@simepy
Copy link
Copy Markdown

@simepy simepy commented Mar 10, 2018

Hello,

This pull request answer to issue #5

I add the shortcut "ALT+q"
When you press shortcut this open iframe below the word or above if iframe have no enough space
I thing make option panel for shortcut in a future version, but I don't understand how it work, at the moment

PS: this is my first pull request, if I do something wrong, just tell me :)

@renanbr
Copy link
Copy Markdown
Owner

renanbr commented Mar 18, 2018

Thank you very much for this PR, this is a feature I appreciate to add since a long time.
Unfortunately, I don't have time for now to give you a decent feedback
I'll get back in some months (before next summer I hope).
Do not hesitate in keep this PR updated and/or make new ones.

@renanbr
Copy link
Copy Markdown
Owner

renanbr commented May 2, 2018

Sorry for the delay.

This patch works great :-)

Thanks to move URLs to HTTPS. I hope this can fix some other reported issues.

I just have some questions about...

... UX:

  • Why Alt+Q?
  • Why do'nt use borders nor shadow to separate the translation box from website?

... structure:

  • What is your intention when declaring web_accessible_resources?
  • Do you have an idea in how to not repeat the pairUrl variable into popup/script.js and popup/contentScript.js?
  • Do you have any reason to keep all files into popup/ folder? (ie.: popup/contentScript.js instead of content-scripts/script.js, and popup/background.js instead of background.js)?

Note: I forget to put .editorconfig file, it was my fault, I'll handle it after

@simepy
Copy link
Copy Markdown
Author

simepy commented May 12, 2018

Hello,

Thanks :)

UX:

  • I had selected Alt+Q completely random... but also because this key is easy to touch but if want to change go ahead :)

  • Yes this is a good idea, but the border is printed before the content this is not good... and I don't know how print the content and if the content is loaded add border

Structure:

  • I add this for export frame.html and add to web page, but It work well without, I remove it
  • Unfortunately not, I search how to fix this but I don't find a solution at the moment
  • No, this is a good idea, I make it

Thanks for your time

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