Просмотр исходного кода

[web] Reapply: Make CIAM login notification dismissible (with cookies) (#30829)

* Reapply "[web] Make CIAM login notification dismissible (with cookies) (#30251)"

This reverts commit 7bafafe54b24245c4da88d1c81540a3b1c98231b.

* Add a test `should redirect to /register with a notification`

* Fix destructuring of options in notification mixins

* Remove `data-ol-dismiss-cookie-paths` default, enforce it being set

* Handle the case of standard notifications without the dismiss setup

Co-authored-by: Jakob Ackermann <jakob.ackermann@overleaf.com>

---------

Co-authored-by: Jakob Ackermann <jakob.ackermann@overleaf.com>
GitOrigin-RevId: fbf441c1efe0aa5d80899a31ec3ad51c1dba6d24
Antoine Clausse 6 месяцев назад
Родитель
Сommit
b57df2602a

+ 12 - 0
services/web/app/src/infrastructure/ExpressLocals.mjs

@@ -337,6 +337,18 @@ export default async function (webRouter, privateApiRouter, publicApiRouter) {
     next()
   })
 
+  webRouter.use(function (req, res, next) {
+    const KEY_PREFIX = 'readnotif-'
+    const dismissedNotifications = []
+    for (const cookieName in req.cookies) {
+      if (cookieName.startsWith(KEY_PREFIX)) {
+        dismissedNotifications.push(cookieName.slice(KEY_PREFIX.length))
+      }
+    }
+    res.locals.dismissedNotifications = dismissedNotifications
+    next()
+  })
+
   webRouter.use(function (req, res, next) {
     res.locals.ExposedSettings = {
       isOverleaf: Settings.overleaf != null,

+ 20 - 10
services/web/app/views/_mixins/ciam_mixins.pug

@@ -109,16 +109,26 @@ mixin ciamErrorNotification
 			.notification-content
 				block
 
-mixin ciamInfoNotification
-	.notification.notification-ds.notification-type-info(
-		role='alert'
-		aria-live='polite'
-	)
-		.notification-icon
-			ph-info(aria-hidden='true')
-		.notification-content-and-cta
-			.notification-content
-				block
+mixin ciamInfoNotification(options)
+	- var {dismissId, dismissCookiePaths} = options ?? {}
+	if dismissId && dismissedNotifications.includes(dismissId)
+		// Notification has been dismissed, do not render
+	else
+		.notification.notification-ds.notification-type-info(
+			role='alert'
+			aria-live='polite'
+			data-ol-dismiss-id=dismissId
+			data-ol-dismiss-cookie-paths=dismissCookiePaths && dismissCookiePaths.join(',')
+		)
+			.notification-icon
+				ph-info(aria-hidden='true')
+			.notification-content-and-cta
+				.notification-content
+					block
+			if dismissId
+				.notification-close-btn
+					button(aria-label=translate('close') data-ol-dismiss-button)
+						ph-x
 
 mixin ciamOrDivider
 	p.ciam-login-register-or-text-container= translate('login_register_or').toUpperCase()

+ 30 - 22
services/web/app/views/_mixins/notification.pug

@@ -12,27 +12,35 @@ mixin notificationIcon(type)
 		+material-symbol('warning')
 
 mixin notification(options)
-	- var {ariaLive, id, type, title, content, disclaimer, className} = options
+	- var {ariaLive, id, type, title, content, disclaimer, className, dismissId, dismissCookiePaths} = options ?? {}
 	- var classNames = `notification notification-type-${type} ${className ? className : ''} ${isActionBelowContent ? 'notification-cta-below-content' : ''}`
 
-	div(aria-live=ariaLive role='alert' id=id class=classNames)
-		.notification-icon
-			+notificationIcon(type)
-		.notification-content-and-cta
-			.notification-content
-				if title
-					p
-						b #{title}
-				| !{content}
-				block
-			//- TODO: handle action
-			//- if action
-			//- 	.notification-cta
-			if disclaimer
-				.notification-disclaimer #{disclaimer}
-		//- TODO: handle dismissible notifications
-		//- TODO: handle onDismiss
-		//- if isDismissible
-		//- 	.notification-close-btn
-		//- 		button(aria-label=translate('close'))
-		//- 			+material-symbol("close")
+	if dismissId && dismissedNotifications.includes(dismissId)
+		//- Notification has been dismissed, do not render
+	else
+		div(
+			aria-live=ariaLive
+			role='alert'
+			id=id
+			class=classNames
+			data-ol-dismiss-id=dismissId
+			data-ol-dismiss-cookie-paths=dismissCookiePaths && dismissCookiePaths.join(',')
+		)
+			.notification-icon
+				+notificationIcon(type)
+			.notification-content-and-cta
+				.notification-content
+					if title
+						p
+							b #{title}
+					| !{content}
+					block
+				//- TODO: handle action
+				//- if action
+				//- 	.notification-cta
+				if disclaimer
+					.notification-disclaimer #{disclaimer}
+			if dismissId
+				.notification-close-btn
+					button(aria-label=translate('close') data-ol-dismiss-button)
+						+material-symbol('close')

+ 1 - 0
services/web/frontend/js/bootstrap.ts

@@ -1,4 +1,5 @@
 import './features/bookmarkable-tab/index'
 import './features/tooltip/index'
 import './features/navbar/index'
+import './features/notifications/dismissible-notifications'
 import 'bootstrap'

+ 1 - 0
services/web/frontend/js/features/form-helpers/form-phosphor-icons.ts

@@ -4,3 +4,4 @@ import '@phosphor-icons/webcomponents/PhEye'
 import '@phosphor-icons/webcomponents/PhInfo'
 import '@phosphor-icons/webcomponents/PhEyeSlash'
 import '@phosphor-icons/webcomponents/PhWarningCircle'
+import '@phosphor-icons/webcomponents/PhX'

+ 49 - 0
services/web/frontend/js/features/notifications/dismissible-notifications.ts

@@ -0,0 +1,49 @@
+import getMeta from '@/utils/meta'
+
+const KEY_PREFIX = 'readnotif-'
+
+function setDismissedNotification(dismissId: string, cookiePaths: string[]) {
+  const name = `${KEY_PREFIX}${dismissId}`
+  const cookieDomain = getMeta('ol-ExposedSettings').cookieDomain
+  const oneYearInSeconds = 60 * 60 * 24 * 365
+
+  for (const path of cookiePaths) {
+    const cookieAttributes =
+      `; path=${path}` +
+      '; domain=' +
+      cookieDomain +
+      '; max-age=' +
+      oneYearInSeconds +
+      '; SameSite=Lax; Secure'
+    document.cookie = `${name}=1;${cookieAttributes}`
+  }
+}
+
+function hydrateDismissibleNotification(notification: HTMLElement) {
+  const dismissId = notification.dataset.olDismissId
+  if (!dismissId) return
+
+  if (!notification.dataset.olDismissCookiePaths) {
+    throw new Error(
+      `Dismissible notifications must have a data-ol-dismiss-cookie-paths attribute ("${dismissId}").`
+    )
+  }
+
+  const dismissCookiePaths =
+    notification.dataset.olDismissCookiePaths.split(',')
+
+  const dismissButton = notification.querySelector<HTMLButtonElement>(
+    '[data-ol-dismiss-button]'
+  )
+
+  if (dismissButton) {
+    dismissButton.addEventListener('click', () => {
+      setDismissedNotification(dismissId, dismissCookiePaths)
+      notification.hidden = true
+    })
+  }
+}
+
+document
+  .querySelectorAll<HTMLElement>('[data-ol-dismiss-id]')
+  .forEach(hydrateDismissibleNotification)

+ 28 - 0
services/web/frontend/stylesheets/ds/components/notification.scss

@@ -27,4 +27,32 @@
   &.notification-type-info {
     background-color: var(--ds-color-blue-50);
   }
+
+  .notification-close-btn {
+    align-self: flex-start;
+    padding: var(--ds-spacing-400) 0 0;
+    height: auto;
+
+    button {
+      @include ds-heading-sm-regular;
+
+      color: var(--ds-color-neutral-950);
+      padding: var(--ds-spacing-150);
+
+      &:hover,
+      &:focus {
+        background-color: var(--ds-color-blue-100);
+      }
+
+      &:active {
+        background-color: var(--ds-color-blue-200);
+      }
+
+      &:focus-visible {
+        background: transparent;
+
+        @include ds-focus-outline;
+      }
+    }
+  }
 }