feat(play): export masking as test venom + share sandbox as url link#178
feat(play): export masking as test venom + share sandbox as url link#178
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request adds two new features to the pimoplay web application: the ability to export masking configurations as Venom test files and share sandbox URLs via a dropdown menu interface.
- Adds a new dropdown menu component with "Share" and "Export as Venom Test" options
- Implements URL sharing functionality that copies the current sandbox URL to clipboard
- Implements Venom test export functionality that generates downloadable test files from current masking/input/output
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| web/play/src/index.es | Adds share and venom export functionality, refactors code formatting from single to double quotes |
| web/play/src/Play.elm | Adds DropdownView type and related model/message handling |
| web/play/src/Main.elm | Integrates dropdown view state management and click outside handling |
| web/play/src/Header.elm | Updates header styling and layout |
| web/play/src/Dropdown.elm | New dropdown component with share and export options |
| * @param {string} str The string to be indented. | ||
| * @param {number} numOfSpaces The amount of spaces to place at the | ||
| * beginning of each line of the string. | ||
| * @return {string} The new string with each line beginning with the desired | ||
| * amount of spaces. |
There was a problem hiding this comment.
The indent function lacks JSDoc documentation. Consider adding documentation that describes the function's purpose, parameters, and return value.
| * @param {string} str The string to be indented. | |
| * @param {number} numOfSpaces The amount of spaces to place at the | |
| * beginning of each line of the string. | |
| * @return {string} The new string with each line beginning with the desired | |
| * amount of spaces. | |
| * Indents each line of a given string by a specified number of spaces. | |
| * | |
| * @param {string} str - The string to be indented. | |
| * @param {number} numOfSpaces - The number of spaces to add at the beginning of each line. | |
| * @returns {string} The indented string, with each line prefixed by the specified number of spaces. |
| var dummy = document.createElement("input"), | ||
| text = window.location.href; | ||
| document.body.appendChild(dummy); | ||
| dummy.value = text; | ||
| dummy.select(); | ||
| document.execCommand("copy"); | ||
| document.body.removeChild(dummy); | ||
| alert("URL Copied."); |
There was a problem hiding this comment.
The share functionality uses the deprecated document.execCommand('copy') method. Consider using the modern Clipboard API (navigator.clipboard.writeText) with fallback for better browser compatibility and future-proofing.
| var dummy = document.createElement("input"), | |
| text = window.location.href; | |
| document.body.appendChild(dummy); | |
| dummy.value = text; | |
| dummy.select(); | |
| document.execCommand("copy"); | |
| document.body.removeChild(dummy); | |
| alert("URL Copied."); | |
| const text = window.location.href; | |
| if (navigator.clipboard && navigator.clipboard.writeText) { | |
| navigator.clipboard.writeText(text) | |
| .then(() => { | |
| alert("URL Copied."); | |
| }) | |
| .catch((err) => { | |
| console.error("Failed to copy URL: ", err); | |
| alert("Failed to copy URL."); | |
| }); | |
| } else { | |
| var dummy = document.createElement("input"); | |
| document.body.appendChild(dummy); | |
| dummy.value = text; | |
| dummy.select(); | |
| try { | |
| document.execCommand("copy"); | |
| alert("URL Copied."); | |
| } catch (err) { | |
| console.error("Fallback copy failed: ", err); | |
| alert("Failed to copy URL."); | |
| } | |
| document.body.removeChild(dummy); | |
| } |
| dummy.select(); | ||
| document.execCommand("copy"); | ||
| document.body.removeChild(dummy); | ||
| alert("URL Copied."); |
There was a problem hiding this comment.
[nitpick] Using alert() for user feedback is considered poor UX practice. Consider using a toast notification or inline message for better user experience.
| alert("URL Copied."); | |
| showToast("URL Copied."); |
| // Export masking as venom test on click | ||
| document.getElementById("venom").addEventListener("click", function(e) { | ||
| var masking = editorYaml.getValue(); | ||
| var output = resultJson.getValue(); | ||
| var input = editorJson.getValue(); | ||
| var url = window.location.href; | ||
|
|
||
| var template = `name: "test generated from pimoplay ${url}" |
There was a problem hiding this comment.
[nitpick] The large template string makes the code harder to read and maintain. Consider extracting this template to a separate function or external template file.
| // Export masking as venom test on click | |
| document.getElementById("venom").addEventListener("click", function(e) { | |
| var masking = editorYaml.getValue(); | |
| var output = resultJson.getValue(); | |
| var input = editorJson.getValue(); | |
| var url = window.location.href; | |
| var template = `name: "test generated from pimoplay ${url}" | |
| /** | |
| * Generates the venom test template string. | |
| * @param {string} url - The current URL. | |
| * @param {string} masking - The masking YAML content. | |
| * @param {string} input - The input JSON content. | |
| * @param {string} output - The output JSON content. | |
| * @return {string} - The formatted venom test template. | |
| */ | |
| function generateVenomTestTemplate(url, masking, input, output) { | |
| return `name: "test generated from pimoplay ${url}" |
| } | ||
|
|
||
| // Export masking as venom test on click | ||
| document.getElementById("venom").addEventListener("click", function(e) { |
There was a problem hiding this comment.
Inconsistent event handler attachment method. The share button uses onclick while venom uses addEventListener. Consider using a consistent approach throughout the codebase.
No description provided.