-
Notifications
You must be signed in to change notification settings - Fork 10
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
Fix missing in layer selection using part of WazeWrap #85
Fix missing in layer selection using part of WazeWrap #85
Conversation
It's an external dependency, so two questions:
IMO we better fix the functionality on our own or using an example with a compatible open license. |
I believe @justins83 created this library to avoid code duplication across scripts, so we could duplicate his code or use the library to avoid duplication. |
Indeed.
Greasyfork is rarely down so in the (just shy of) 2 years that it has been
in use there has never been a problem.
I am actually going to be changing WazeWrap to be an installable script
rather than work as a dependency, however. The reason for this is due to
how tampermonkey uses dependencies - every script that uses it pulls down
its own copy, but they all overwrite the same variable on `window`, so the
last one to load always "wins", but they still exhibit some behavior of
operating on different versions. With some of the changes that I have
planned for WW it starts becoming difficult to update due to some scripts
targeting specific versions of WW through the `?version=` URL parameter,
and because of how Tampermonkey checks for dependency updates they can be
out of date for a week after updating before the script pulls the latest
version.
…On Mon, Nov 5, 2018 at 8:40 AM David Westerink ***@***.***> wrote:
1. I don't know, I haven't build this as a Chrome extension. How do I
do that?
2. A lot of other scripts won't work then either.
I believe @justins83 <https://github.com/justins83> created this library
to avoid code duplication across scripts, so we could duplicate his code or
use the library to avoid duplication.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#85 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ATTy2rtseBf5yq1vC2Rfffk9_iUhkNgiks5usD-9gaJpZM4YOTwv>
.
--
JustinS83
USA Local Champ
|
Sure, code duplication must be avoided. If WazeWrap is on GitHub, we can try to use it as a subrepo. This might duplicate the code in the browser, but there will be no external dependency and will be just one source repo. What do you think? |
WazeWrap writes everything to an object on |
So basically, there will be no other way to use WazeWrap, unless we ask users to install it prior to the Validator? |
Yes. There have been too many issues with some script specifying versions
or tampermonkey not pulling the latest dependency because it only forces
that when the script gets a version number bump, which then requires
bumping the version on a dozen scripts just so they use the latest version
of the dependency.
As of right now there are over 40 scripts (that I know of) that are using
WazeWrap, so chances are it will be installed.
…On Mon, Nov 5, 2018 at 10:24 AM Andriy Berestovskyy < ***@***.***> wrote:
So basically, there will be no other way to use WazeWrap, unless we ask
users to install it prior to the Validator?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#85 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ATTy2ok07Sk4QSurUpa9jKnP2BjNple6ks5usFg7gaJpZM4YOTwv>
.
--
JustinS83
USA Local Champ
|
What's the license for the WazeWrap? Are we allowed to reuse some fragments in Validator? |
If the license for WazeWrap allows it, copying the code over to the lib of Validator isn't a problem. |
@justins83 so, can we use a part of WazeWrap in Validator? @BellHouse maybe you can contribute a piece of code to add a layer in WME? Whatever you have, we will adapt it to our needs... |
Sorry guys, work kicked my butt this week. WazeWrap currently doesn't have a license, which technically means myself and MoM are the only ones allowed to use it. I'll have to find a license for it. You are welcome to use the add layer checkbox method - it works very well and handles changing to/from MTE mode and changing imperial/metric. `
` |
The |
Sorry, I am not quite sure I understand you. There is no shortcut class in the code posted by Justin, do you refer WazeWrap? Overall, if the shortcut is an issue, let's just have the layer for now? |
This fixes the shortcut not working for us. There is a lot of code involved for adding the shortcut in a good way, so I propose using WazeWrap for add the shortcut. In the future we could utilize WazeWrap for more things?
This fixes WMEValidator#59 The layer is added to the layer selection and toggles with the shortcut
Extracting the WazeWrap function to add a layer checkbox. Not fixing the shortcut for now
c79e7f7
to
7fd7c41
Compare
Okay, I've updated the PR so it now only fixes the layer that is missing from the layer selector. |
@davidakachaos the WazeWrap has no license, so it's not open-source as defined by Wikipedia. Please ask Justin explicitly if we need something besides the code snippet posted here. Closure Compiler supports ES6. Try to change the line |
This fixes the shortcut not working for us. There is a lot of code involved for adding the shortcut in a good way, so I propose using WazeWrap for add the shortcut.
Also for #59 this uses WazeWrap to add the layer to the layer selector, and toggles with the shortcut.