Skip to content
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

Make notifications permanently dismissed #283

Merged
merged 13 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions dist/readthedocs-addons.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/readthedocs-addons.js.map

Large diffs are not rendered by default.

88 changes: 68 additions & 20 deletions src/notification.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ export class NotificationElement extends LitElement {
config: { state: true },
urls: { state: true },
highest_version: { state: true },
dismissedTimestamp: { state: true },
localStorageKey: { state: true },
// Under which key should the notification store dismissal (and other) information
notificationStorageKey: {
type: String,
attribute: "notification-storage-key",
},
};

/** @static @property {Object} - Lit stylesheets to apply to elements */
Expand All @@ -37,6 +44,29 @@ export class NotificationElement extends LitElement {
this.readingLatestVersion = false;
this.readingStableVersion = false;
this.stableVersionAvailable = false;
// This will store information like user dimissing the notification. Any Notification sharing
// the same localStorageKey will be affected. If a specific Notification should not be dismissed after
// another has been dismissed, it requires a different localStorageKey
this.localStorageKey =
this.notificationStorageKey || "default-notification";
zanderle marked this conversation as resolved.
Show resolved Hide resolved
this.dismissedTimestamp = null;
}

connectedCallback() {
super.connectedCallback();
this.checkDismissedTimestamp();
}
humitos marked this conversation as resolved.
Show resolved Hide resolved

checkDismissedTimestamp() {
// Check if this notification (as determined by localStorageKey) has been dismissed already.
// Once a notification has been dismissed, it stays dismissed. This information however is not passed
// over different subdomains, so if a notification has been dismissed on a PR build, it will not affect
// other builds.
const notificationStorage =
NotificationAddon.getLocalStorage()[this.localStorageKey];
humitos marked this conversation as resolved.
Show resolved Hide resolved
if (notificationStorage && notificationStorage.dismissedTimestamp) {
this.dismissedTimestamp = notificationStorage.dismissedTimestamp;
}
}

loadConfig(config) {
Expand Down Expand Up @@ -71,6 +101,17 @@ export class NotificationElement extends LitElement {
) {
this.calculateStableLatestVersionWarning();
}
this.localStorageKey =
this.notificationStorageKey ||
this.getLocalStorageKeyFromConfig(this.config);
this.checkDismissedTimestamp();
}

getLocalStorageKeyFromConfig(config) {
const projectSlug = config.projects.current.slug;
const languageCode = config.projects.current.language.code;
const versionSlug = config.versions.current.slug;
return `${projectSlug}-${languageCode}-${versionSlug}-notification`;
}

firstUpdated() {
Expand All @@ -86,6 +127,11 @@ export class NotificationElement extends LitElement {
return nothing;
}

// This notification has been dimissed, so don't render it
if (this.dismissedTimestamp) {
return nothing;
}

if (this.config.versions.current.type === "external") {
if (this.config.addons.external_version_warning.enabled) {
return this.renderExternalVersionWarning();
Expand Down Expand Up @@ -145,12 +191,8 @@ export class NotificationElement extends LitElement {
}

renderStableLatestVersionWarning() {
library.add(faCircleXmark);
library.add(faHourglassHalf);
library.add(faFlask);
const xmark = icon(faCircleXmark, {
title: "Close notification",
});
if (this.readingLatestVersion && this.stableVersionAvailable) {
const iconFlask = icon(faFlask, {
classes: ["header", "icon"],
Expand All @@ -161,9 +203,7 @@ export class NotificationElement extends LitElement {
${iconFlask.node[0]}
<div class="title">
This is the <span>latest development version</span>
<a href="#" class="right" @click=${this.closeNotification}>
${xmark.node[0]}
</a>
${this.renderCloseButton()}
</div>
<div class="content">
Some features may not yet be available in the published stable
Expand All @@ -187,9 +227,7 @@ export class NotificationElement extends LitElement {
<div class="title">
This <em>may</em> be an
<span>old version of this documentation</span>
<a href="#" class="right" @click=${this.closeNotification}>
${xmark.node[0]}
</a>
${this.renderCloseButton()}
</div>
<div class="content">
You may be reading an old version of this documentation. Read the
Expand All @@ -205,11 +243,7 @@ export class NotificationElement extends LitElement {
}

renderExternalVersionWarning() {
library.add(faCircleXmark);
library.add(faCodePullRequest);
const xmark = icon(faCircleXmark, {
title: "Close notification",
});
const iconPullRequest = icon(faCodePullRequest, {
title: "This version is a pull request version",
classes: ["header", "icon"],
Expand All @@ -220,9 +254,7 @@ export class NotificationElement extends LitElement {
${iconPullRequest.node[0]}
<div class="title">
This page was created from a pull request build
<a href="#" class="right" @click=${this.closeNotification}>
${xmark.node[0]}
</a>
${this.renderCloseButton()}
</div>
<div class="content">
See the
Expand All @@ -239,13 +271,29 @@ export class NotificationElement extends LitElement {
`;
}

renderCloseButton() {
library.add(faCircleXmark);
const xmark = icon(faCircleXmark, {
title: "Close notification",
});
return html`
<a href="#" class="right" @click=${this.closeNotification}>
${xmark.node[0]}
</a>
`;
}

closeNotification(e) {
// Avoid going back to the top of the page when closing the notification
e.preventDefault();

// TODO add cookie to allow closing this notification for all page views on this
// PR build.
this.remove();
// Store the information about dismissal in the Local Storage
// Make sure to store the timestamp, so that we have the option to maybe add a TTL on the dismissal
this.dismissedTimestamp = Date.now();
const dismissedObj = {
[this.localStorageKey]: { dismissedTimestamp: this.dismissedTimestamp },
};
NotificationAddon.setLocalStorage(dismissedObj);

// Avoid event propagation
return false;
Expand Down
31 changes: 31 additions & 0 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ export class AddonBase {
static jsonValidationURI = null;
static addonName = null;
static addonEnabledPath = null;
// The key to be used for Local Storage purposes. If not provided, `readthedocs-<addonName>-storage-key`
// will be used
static addonLocalStorageKey = null;

static isConfigValid(config) {
const validate = ajv.getSchema(this.jsonValidationURI);
Expand All @@ -79,13 +82,41 @@ export class AddonBase {
);
}

static getAddonLocalStorageKey() {
// Return a key to be used for Local Storage
return (
this.addonLocalStorageKey || `readthedocs-${this.addonName}-storage-key`
);
}

static requiresUrlParam() {
// Decide whether or not this addons requires sending `url=` parameter to the API endpoint.
// Sending this attribute will make the API response to contain extra data (e.g. resolved URLs that depend on the exact URL)
//
// Note that sending `url=` attribute reduces the possibilities to use a cached response accross all the pages for the same project/version.
return false;
}

static getLocalStorage() {
// Get the object stored in Local Storage for this addon
const addonLocalStorage = window.localStorage.getItem(
this.getAddonLocalStorageKey(),
);
return addonLocalStorage ? JSON.parse(addonLocalStorage) : {};
}

static setLocalStorage(obj) {
// Update the existing object in Local Storage with the provided object and store it under
// the key for this addon.
// For example, if obj provided is {foo: 'bar'}, and the existing object is {a: 1, foo: 'baz'}
// This will update the existing object to {a: 1, foo: 'bar'} and store it under this.getAddonLocalStorageKey()
const addonLocalStorage = this.getLocalStorage() || {};
const updatedStorage = { ...addonLocalStorage, ...obj };
window.localStorage.setItem(
this.getAddonLocalStorageKey(),
JSON.stringify(updatedStorage),
);
}
}

/**
Expand Down
91 changes: 91 additions & 0 deletions tests/notification.test.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,29 @@
<script type="module">
import { expect, elementUpdated } from "@open-wc/testing";
import { runTests } from "@web/test-runner-mocha";
import { stub } from "sinon";
import * as notification from "../src/notification.js";

// Define the config globally to be able to override on ``beforeEach`` and inside each test
let config;

// stub localStorage methods, to ensure test isolation
const getLocalStorageStub = stub(window.localStorage, "getItem");
const setLocalStorageStub = stub(window.localStorage, "setItem");

runTests(async () => {
beforeEach(() => {
// Check if there are any existing notifications in the HTML and remove them
// as they might contain state from the previous tests
const existingNotification = document.querySelector(
"readthedocs-notification",
);
if (existingNotification) {
existingNotification.remove();
}
// Reset localStorage stubs
getLocalStorageStub.reset();
setLocalStorageStub.reset();
config = {
addons: {
external_version_warning: {
Expand Down Expand Up @@ -197,6 +213,81 @@

expect(element).to.have.attribute("class", "raised floating");
});

it("doesn't render when closed", async () => {
config.versions.current.slug = "latest";

const addon = new notification.NotificationAddon(config);
const element = document.querySelector("readthedocs-notification");

// We need to wait for the element to renders/updates before querying it
await elementUpdated(element);

const closeButton = element.shadowRoot.querySelector("a.right");
closeButton.click();
await elementUpdated(element);

// The notification should not be displayed anymore
expect(element).shadowDom.to.equal("");

// localStorage should now contain information about the dismissal
const key = setLocalStorageStub.getCall(0).args[0];
const value = setLocalStorageStub.getCall(0).args[1];
expect(key).to.be.equal("readthedocs-Notification-storage-key");
zanderle marked this conversation as resolved.
Show resolved Hide resolved
const valueObj = JSON.parse(value);
expect(valueObj).to.have.property("project-en-latest-notification");
expect(valueObj["project-en-latest-notification"]).to.have.property(
"dismissedTimestamp",
);
});

it("doesn't render when previously closed", async () => {
config.versions.current.slug = "latest";

// localStorage should contain information about the previous dismissal
const addonInformation = {
"project-en-latest-notification": { dismissedTimestamp: 123 },
};
const localStorageString = JSON.stringify(addonInformation);
getLocalStorageStub
.withArgs("readthedocs-Notification-storage-key")
Copy link
Member

@humitos humitos Jun 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to use addon.getLocalStorageKeyFromConfig(config) here (and other places) instead of hardcoding it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to hardcode it, because this way we also test that we don't get some accidental readthedocs-undefined-storage-key or something like that.

.returns(localStorageString);

const addon = new notification.NotificationAddon(config);
const element = document.querySelector("readthedocs-notification");

// We need to wait for the element to renders/updates before querying it
await elementUpdated(element);

// The notification should not be displayed
expect(element).shadowDom.to.equal("");
});

it("renders when a different version was previously closed", async () => {
config.versions.current.slug = "381";
config.versions.current.type = "external";

// localStorage should contain information about the previous dismissal
const addonInformation = {
"project-en-v1.0-notification": { dismissedTimestamp: 123 },
};
const localStorageString = JSON.stringify(addonInformation);
getLocalStorageStub
.withArgs("readthedocs-Notification-storage-key")
.returns(localStorageString);

const addon = new notification.NotificationAddon(config);
const element = document.querySelector("readthedocs-notification");

// We need to wait for the element to renders/updates before querying it
await elementUpdated(element);

// The notification should be displayed normally
const title = element.shadowRoot.querySelector("div.title");
expect(title.innerText).to.be.equal(
"This page was created from a pull request build",
);
});
});
});
</script>
Expand Down