- 
                Notifications
    You must be signed in to change notification settings 
- Fork 14
feat(play): export masking as test venom + share sandbox as url link #178
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
base: main
Are you sure you want to change the base?
feat(play): export masking as test venom + share sandbox as url link #178
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.
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. | 
    
      
    
      Copilot
AI
    
    
    
      Jul 18, 2025 
    
  
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.
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."); | 
    
      
    
      Copilot
AI
    
    
    
      Jul 18, 2025 
    
  
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.
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."); | 
    
      
    
      Copilot
AI
    
    
    
      Jul 18, 2025 
    
  
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.
[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}" | 
    
      
    
      Copilot
AI
    
    
    
      Jul 18, 2025 
    
  
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.
[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) { | 
    
      
    
      Copilot
AI
    
    
    
      Jul 18, 2025 
    
  
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.
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.