EandrewJones commented on code in PR #480:
URL: https://github.com/apache/flagon-useralejs/pull/480#discussion_r1684802556
##########
src/UserALEWebExtension/optionsPage.html:
##########
@@ -50,7 +50,39 @@ <h1>Options</h1>
<br />
<div align="right">
- <button type="submit" id="submit">Save</button>
+ <button type="submit" id="submitOptions">Save Changes</button>
+ </div>
+ </form>
+ <h1>Report an Issue</h1>
+ <form name="issueForm" id="issueForm">
+ <label>Issue Type</label><br />
+ <label>
+ <input
+ type="radio"
+ id="bug"
+ name="issueType"
+ value="Bug"
+ checked="checked"
+ />
+ Bug </label
+ ><br />
+
+ <label>
+ <input type="radio" id="feat" name="issueType" value="Feature" />
+ Feature Request </label
+ ><br />
+ <br />
Review Comment:
The ol' `<br/>` to serve as padding instead of proper styles ;). Will let it
slide for now.
##########
src/UserALEWebExtension/icons/UserALE Logo.png:
##########
Review Comment:
Are we legally allowed to use the Apache feather in this logo?
##########
src/UserALEWebExtension/options.ts:
##########
@@ -76,7 +75,46 @@ function getConfig() {
(document.getElementById("filter") as HTMLInputElement).value =
payload.pluginConfig.urlWhitelist;
});
+
+ (document.getElementById("optionsForm") as HTMLFormElement).addEventListener(
+ "submit",
+ setConfig,
+ );
+ (document.getElementById("issueForm") as HTMLFormElement).addEventListener(
+ "submit",
+ reportIssue,
+ );
+}
+
+function reportIssue() {
+ browser.runtime.sendMessage({
+ type: MessageTypes.ISSUE_REPORT,
+ payload: {
+ issueType: (
+ document.querySelector(
+ 'input[name="issueType"]:checked',
+ ) as HTMLButtonElement
+ ).value,
+ issueDescription: (
+ document.getElementById("issueDescription") as HTMLTextAreaElement
+ ).value,
+ type: "issue",
Review Comment:
Do we want to add these as additional fields or put them in the "details"
field? If I recall, you added a discussion ticket for us to standardize our
usage of details based on some conventions.
Right now it contains additional, action-specific contextual information for
certain event types. Feels like a natural place to include these fields.
##########
src/UserALEWebExtension/messageTypes.ts:
##########
@@ -20,3 +20,4 @@ const prefix = "USERALE_";
export const CONFIG_CHANGE = prefix + "CONFIG_CHANGE";
export const ADD_LOG = prefix + "ADD_LOG";
export const HTTP_SESSION = prefix + "HTTP_SESSION";
+export const ISSUE_REPORT = prefix + "ISSUE_REPORT";
Review Comment:
I recommend we use an Enum here. That's essentially how you're using the
import in other locations anyways.
##########
package.json:
##########
@@ -16,6 +16,7 @@
"test": "npm run test:unit && npm run test:e2e",
"test:unit": "jest -c ./test/unit/jest.config.ts",
"test:e2e": "playwright test -c ./test/e2e/playwright.config.ts",
+ "pretest:e2e": "rm -rf ./test/e2e/chromeData/*",
"posttest:e2e": "rm -rf ./test/e2e/chromeData/*",
Review Comment:
Why do we need to clean before and after?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@flagon.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@flagon.apache.org
For additional commands, e-mail: notifications-h...@flagon.apache.org
