-
Notifications
You must be signed in to change notification settings - Fork 1k
Open
Labels
HTMLrequires change to HTML filesrequires change to HTML fileshelp neededWe'd love volunteers to advise on or help fix/implement this.We'd love volunteers to advise on or help fix/implement this.javascriptrequires change to JavaScript filesrequires change to JavaScript files
Description
We have a Collapsible Controller that sets cookies with the prefix callout_block_
.
warehouse/warehouse/static/js/warehouse/controllers/collapsible_controller.js
Lines 16 to 69 in 1bb4024
export default class extends Controller { | |
/** | |
* Get element's collasped status from the cookie. | |
* @private | |
*/ | |
_getCollapsedCookie() { | |
const id = this.data.get("identifier"); | |
const value = document.cookie.split(";").find(item => item.startsWith(`callout_block_${id}_collapsed=`)); | |
return value ? value.split("=")[1] : null; | |
} | |
/** | |
* Set element's collapsed status as a cookie. | |
* @private | |
*/ | |
_setCollapsedCookie(value) { | |
if (this.data.get("setting") === "global") | |
document.cookie = `callout_block_${this.data.get("identifier")}_collapsed=${value};path=/`; | |
else | |
document.cookie = `callout_block_${this.data.get("identifier")}_collapsed=${value}`; | |
} | |
initialize() { | |
switch (this._getCollapsedCookie()) { | |
case "1": | |
this.collapse(); | |
break; | |
case "0": | |
this.expand(); | |
break; | |
default: | |
this.save(); | |
} | |
} | |
collapse() { | |
this.element.removeAttribute("open"); | |
this._setCollapsedCookie("1"); | |
} | |
expand() { | |
this.element.setAttribute("open", ""); | |
this._setCollapsedCookie("0"); | |
} | |
save() { | |
setTimeout(() => { | |
if (!this.element.hasAttribute("open")) | |
this._setCollapsedCookie("1"); | |
else | |
this._setCollapsedCookie("0"); | |
}, 0); | |
} | |
} |
Similar code exists in Dismissable Controller.
This code uses cookies, and may be better served by using localStorage
instead. See Notification Controller for how this is done.
warehouse/warehouse/static/js/warehouse/controllers/notification_controller.js
Lines 17 to 59 in 1bb4024
export default class extends Controller { | |
static targets = ["notification", "notificationDismiss"]; | |
/** | |
* Get notification's id based on current DOM element id and version | |
* | |
* Notifications _without_ an element id and `notification-data-version` | |
* will be treated as ephemeral: its dismissed state will not be persisted | |
* into localStorage. | |
* | |
* @private | |
*/ | |
_getNotificationId() { | |
/** Get data from `data-notification-version` attribute */ | |
if (this.notificationTarget.id) { | |
const version = this.data.get("version") || "-1"; | |
return `${this.notificationTarget.id}_${version}__dismissed`; | |
} | |
return null; | |
} | |
initialize() { | |
const notificationId = this._getNotificationId(); | |
const isDismissable = this.notificationTarget.classList.contains("notification-bar--dismissable"); | |
// Show the notification if: | |
// - the notification is ephemeral, i.e. it has no notification ID | |
// - it's not ephemeral and is not dismissable | |
// - it's not ephemeral, is dismissable and the user has not dismissed it yet | |
if (!notificationId || (isDismissable && !localStorage.getItem(notificationId))) { | |
this.notificationTarget.classList.add("notification-bar--visible"); | |
} | |
} | |
dismiss() { | |
const notificationId = this._getNotificationId(); | |
if (notificationId) { | |
localStorage.setItem(notificationId, 1); | |
} | |
this.notificationTarget.classList.remove("notification-bar--visible"); | |
positionWarning(); | |
} | |
} |
(p.s. the notification keys could use some polish so they have recognizable names )
There may also be an opportunity here to convert all three to use a similar controller, and collapse (pun!) all three into one.
Metadata
Metadata
Assignees
Labels
HTMLrequires change to HTML filesrequires change to HTML fileshelp neededWe'd love volunteers to advise on or help fix/implement this.We'd love volunteers to advise on or help fix/implement this.javascriptrequires change to JavaScript filesrequires change to JavaScript files