Skip to content

Conversation

@giraud10
Copy link
Contributor

No description provided.

@giraud10 giraud10 linked an issue Oct 21, 2022 that may be closed by this pull request
@adrienaury adrienaury requested a review from Copilot July 18, 2025 12:25
Copy link
Contributor

Copilot AI left a 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

Comment on lines +212 to +216
* @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.
Copy link

Copilot AI Jul 18, 2025

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.

Suggested change
* @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.

Copilot uses AI. Check for mistakes.
Comment on lines +198 to +205
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.");
Copy link

Copilot AI Jul 18, 2025

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.

Suggested change
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);
}

Copilot uses AI. Check for mistakes.
dummy.select();
document.execCommand("copy");
document.body.removeChild(dummy);
alert("URL Copied.");
Copy link

Copilot AI Jul 18, 2025

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.

Suggested change
alert("URL Copied.");
showToast("URL Copied.");

Copilot uses AI. Check for mistakes.
Comment on lines +223 to +230
// 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}"
Copy link

Copilot AI Jul 18, 2025

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.

Suggested change
// 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}"

Copilot uses AI. Check for mistakes.
}

// Export masking as venom test on click
document.getElementById("venom").addEventListener("click", function(e) {
Copy link

Copilot AI Jul 18, 2025

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.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[PROPOSAL] export a pimo play sandbox as venom test

2 participants