Skip to content

Commit

Permalink
Automagically hide butter bars (#150)
Browse files Browse the repository at this point in the history
This is a follow up to #146 because I overlooked the following
acceptance criteria:
`The warning disappears on its own when all proxy-setting extensions are
inactive / removed.`

Ergo, this PR:
- Automagically hides butter bar alerts when conflicting extensions are
disabled and removed
- Does not add these alerts to the list of dismissed alerts so that they
are eligible to be shown again, should the same conflict or another
conflict re-materialize
- Renames the fn exposed to the UI from `removeAlert` -> `dismissAlert` 
- Dismissed alerts _are_ added to the dismissed alert list and are not
shown again, regardless of the status of the conflict.

- Updates and adds tests
  • Loading branch information
lesleyjanenorton authored Dec 14, 2024
1 parent 20bc5ea commit 40a3036
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 9 deletions.
31 changes: 26 additions & 5 deletions src/background/butterBarService.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export class ButterBarService extends Component {
// Gets exposed to UI
static properties = {
butterBarList: PropertyType.Bindable,
removeAlert: PropertyType.Function,
dismissAlert: PropertyType.Function,
};

/** @type {IBindable<Array<ButterBarAlert>>} */
Expand Down Expand Up @@ -73,19 +73,29 @@ export class ButterBarService extends Component {
* @param {ButterBarAlert} alert
*/
maybeCreateAlert(list, alert) {
const { alertId } = alert;
const alertInButterBarList = this.alertInButterBarList(
alertId,
this.butterBarList.value
);

if (list.length == 0) {
if (!alertInButterBarList) {
return;
}

this.removeAlert(alertId);
return;
}
const { alertId } = alert;

if (
this.alertWasDismissed(alertId, this.dismissedAlerts) ||
this.alertInButterBarList(alertId, this.butterBarList.value)
) {
return;
}

return this.butterBarList.value.push(alert);
this.butterBarList.set([...this.butterBarList.value, alert]);
return;
}

/**
Expand All @@ -104,14 +114,25 @@ export class ButterBarService extends Component {
return butterBarList.some((alert) => alert.alertId == id);
}

removeAlert(id) {
// Called from the UI when a user has dismissed the butter bar
dismissAlert(id) {
const newAlertList = this.butterBarList.value.filter(
({ alertId }) => alertId !== id
);
this.dismissedAlerts.push(id);
this.butterBarList.set(newAlertList);
return;
}

// Removes an alert from the butter bar list without adding
// it to the list of dismissed alerts.
removeAlert(id) {
const newAlertList = this.butterBarList.value.filter(
({ alertId }) => alertId !== id
);
this.butterBarList.set([...newAlertList]);
return;
}
}

export class ButterBarAlert {
Expand Down
5 changes: 5 additions & 0 deletions src/background/conflictObserver.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ export class ConflictObserver {

constructor() {
this.updateList();

browser.management.onInstalled.addListener(this.updateList.bind(this));
browser.management.onUninstalled.addListener(this.updateList.bind(this));
browser.management.onEnabled.addListener(this.updateList.bind(this));
browser.management.onDisabled.addListener(this.updateList.bind(this));
}

async updateList() {
Expand Down
6 changes: 3 additions & 3 deletions src/components/butter-bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ export class ButterBar extends LitElement {
window.close();
};

const removeAlert = (id) => {
butterBarService.removeAlert(id);
const dismissAlert = (id) => {
butterBarService.dismissAlert(id);
this.dispatchEvent(new CustomEvent("resize-popup", { bubbles: true }));
};

Expand All @@ -49,7 +49,7 @@ export class ButterBar extends LitElement {
</div>
<button
@click=${() => {
removeAlert(this.alertId);
dismissAlert(this.alertId);
}}
class="butter-bar-close ghost-btn"
>
Expand Down
52 changes: 51 additions & 1 deletion tests/jest/background/butterBarService.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe("ButterBarService", () => {
butterBarService.maybeCreateAlert(list, testButterBarAlert);
expect(butterBarService.butterBarList.value.length).toBe(1);

butterBarService.removeAlert("new-alert");
butterBarService.dismissAlert("new-alert");
expect(butterBarService.butterBarList.value.length).toBe(0);
});

Expand All @@ -61,6 +61,56 @@ describe("ButterBarService", () => {
expect(butterBarService.butterBarList.value.length).toBe(1);
});

test("Alerts are removed when the conflict that triggered them is gone", () => {
const conflictObserver = new TestConflictObserver();
const butterBarService = new ButterBarService(
new TestRegister(),
conflictObserver
);

let list = [1];
butterBarService.maybeCreateAlert(list, testButterBarAlert);
expect(butterBarService.butterBarList.value.length).toBe(1);

list = [];
butterBarService.maybeCreateAlert(list, testButterBarAlert);
expect(butterBarService.butterBarList.value.length).toBe(0);
});

test("Alerts are only added to the dismissed list when dismissed from the UI", () => {
const conflictObserver = new TestConflictObserver();
const butterBarService = new ButterBarService(
new TestRegister(),
conflictObserver
);

let list = [1];
butterBarService.maybeCreateAlert(list, testButterBarAlert);
expect(butterBarService.butterBarList.value.length).toBe(1);

list = [];
butterBarService.removeAlert(testButterBarAlert.alertId);
expect(butterBarService.dismissedAlerts.length).toBe(0);
});

test("Removed (but not dismissed) alerts are shown in the UI if the same conflict resurfaces", () => {
const conflictObserver = new TestConflictObserver();
const butterBarService = new ButterBarService(
new TestRegister(),
conflictObserver
);

let list = [1];
butterBarService.maybeCreateAlert(list, testButterBarAlert);
expect(butterBarService.butterBarList.value.length).toBe(1);

butterBarService.removeAlert(testButterBarAlert.alertId);
expect(butterBarService.dismissedAlerts.length).toBe(0);

butterBarService.maybeCreateAlert([1], testButterBarAlert);
expect(butterBarService.butterBarList.value.length).toBe(1);
});

test("ButterBarService.alertWasDismissed returns true if the ID is in the provided list", () => {
const conflictObserver = new TestConflictObserver();
const butterBarService = new ButterBarService(
Expand Down
25 changes: 25 additions & 0 deletions tests/jest/background/conflictTest.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,18 @@ import { beforeEach, describe, expect, test, jest } from "@jest/globals";

// Mock the browser API
const mockGetAll = jest.fn();
const mockOnInstalled = { addListener: jest.fn() };
const mockOnUninstalled = { addListener: jest.fn() };
const mockOnEnabled = { addListener: jest.fn() };
const mockOnDisabled = { addListener: jest.fn() };

global.browser = {
management: {
getAll: mockGetAll,
onInstalled: mockOnInstalled,
onUninstalled: mockOnUninstalled,
onEnabled: mockOnEnabled,
onDisabled: mockOnDisabled,
},
};
/**
Expand Down Expand Up @@ -40,6 +48,23 @@ describe("ConflictObserver", () => {
mockGetAll.mockReset();
});

it("should initialize and set up event listeners", () => {
const observer = new ConflictObserver();

expect(mockOnInstalled.addListener).toHaveBeenCalledWith(
expect.any(Function)
);
expect(mockOnUninstalled.addListener).toHaveBeenCalledWith(
expect.any(Function)
);
expect(mockOnEnabled.addListener).toHaveBeenCalledWith(
expect.any(Function)
);
expect(mockOnDisabled.addListener).toHaveBeenCalledWith(
expect.any(Function)
);
});

it("Calls getAll on creation", async () => {
mockGetAll.mockResolvedValue([]);
const test = new ConflictObserver();
Expand Down

0 comments on commit 40a3036

Please sign in to comment.