-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Added 'More Info' links to the descriptions of many operations for #265 #298
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@PenguinGeorge thanks for this. You may have seen on PR #284 that we are now freezing the master branch until all operations are ported over to use ESM. If you have any time, you could consider branching off the esm
branch and making these changes off that. This will make it easier to merge later.
Other notes:
- The test suite is failing on this branch. This is due to some errors in the operationConfig - you've mistyped a few
inputType
properties when adding in the new infoURLs. This should be a quick fix. - Consider using template literals for your html templates. It's not essential, but it may help make it a little cleaner.
src/core/config/OperationConfig.js
Outdated
inputType: "ArrayBuffer", | ||
outputType: "string", | ||
args: [] | ||
}, | ||
"MD5": { | ||
module: "Hashing", | ||
description: "MD5 (Message-Digest 5) is a widely used hash function. It has been used in a variety of security applications and is also commonly used to check the integrity of files.<br><br>However, MD5 is not collision resistant and it isn't suitable for applications like SSL/TLS certificates or digital signatures that rely on this property.", | ||
inputType: "ArrayBuffer", | ||
infoURL: "https://wikipedia.org/wiki/MD5", | ||
nputType: "ArrayBuffer", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This kind of error is what is causing the tests to fail
Thanks very much for this @PenguinGeorge. I transferred all your links over to the new operation format and added more links for most of the remaining operations. I also modified the display code so that it pulls the Wikipedia page title out of the URL, making it a bit clearer to the user. The Thanks once again, this is really helpful. |
[FEATURE] Interactive Config Editor It is merged, the new config editor is finally here!! 🎉
Should resolve issue #265.
This adds a new optional
infoURL
field to operations in OperationConfig.js, which includes a URL to a relevant Wikipedia article. The URLs are generic (i.e. not localised) Wikipedia links, so will automatically redirect to the user's language.When a description popover is displayed, and when a URL has been provided for the operation, a link will be displayed at the bottom of the popover, as seen in the following example:
I thought it would be best to add the
infoURL
field rather than trying to parse the description looking for links, as this would be less reliable.It is also worth noting that should this PR be merged, the Wiki article for adding operations should be updated to include the new optional field.