Merge pull request #3973 from overleaf/as-clean-up-hue-generator

Clean up hue generator

GitOrigin-RevId: 1b7fad05d7c2f6519efbbe0379f2449c273e4f4b

services/web/frontend/js/features/chat/components/message.js

@@ -1,11 +1,11 @@
 import React from 'react'
 import PropTypes from 'prop-types'
-import ColorManager from '../../../ide/colors/ColorManager'
+import { getHueForUserId } from '../../../shared/utils/colors'
 import MessageContent from './message-content'
 
 function Message({ message, userId }) {
   function hue(user) {
-    return user ? ColorManager.getHueForUserId(user.id) : 0
+    return user ? getHueForUserId(user.id, userId) : 0
   }
 
   function getMessageStyle(user) {

services/web/frontend/js/features/editor-navigation-toolbar/components/online-users-widget.js

@@ -3,7 +3,7 @@ import PropTypes from 'prop-types'
 import { useTranslation } from 'react-i18next'
 import { Dropdown, MenuItem, OverlayTrigger, Tooltip } from 'react-bootstrap'
 import Icon from '../../../shared/components/icon'
-import ColorManager from '../../../ide/colors/ColorManager'
+import { getHueForUserId } from '../../../shared/utils/colors'
 
 function OnlineUsersWidget({ onlineUsers, goToUser }) {
   const { t } = useTranslation()
@@ -64,9 +64,7 @@ OnlineUsersWidget.propTypes = {
 }
 
 function UserIcon({ user, showName, onClick }) {
-  const backgroundColor = `hsl(${ColorManager.getHueForUserId(
-    user.user_id
-  )}, 70%, 50%)`
+  const backgroundColor = `hsl(${getHueForUserId(user.user_id)}, 70%, 50%)`
 
   function handleOnClick() {
     onClick(user)

services/web/frontend/js/ide/colors/ColorManager.js

@@ -13,6 +13,9 @@
  * DS207: Consider shorter variations of null checks
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
+
+// NOTE: this file is being refactored over to frontend/js/shared/utils/colors.js
+
 import CryptoJS from 'crypto-js/md5'
 let ColorManager
 

services/web/frontend/js/shared/utils/colors.js

@@ -0,0 +1,38 @@
+import md5 from 'crypto-js/md5'
+
+const ANONYMOUS_HUE = 100
+const OWN_HUE = 200 // We will always appear as this color to ourselves
+const OWN_HUE_BLOCKED_SIZE = 20 // no other user should have a HUE in this range
+const TOTAL_HUES = 360 // actually 361, but 360 for legacy reasons
+
+export function getHueForUserId(userId, currentUserId) {
+  if (userId == null || userId === 'anonymous-user') {
+    return ANONYMOUS_HUE
+  }
+
+  if (currentUserId === userId) {
+    return OWN_HUE
+  }
+
+  let hue = getHueForId(userId)
+
+  // if `hue` is within `OWN_HUE_BLOCKED_SIZE` degrees of the personal hue
+  // (`OWN_HUE`), shift `hue` to the end of available hues by adding
+  if (
+    hue > OWN_HUE - OWN_HUE_BLOCKED_SIZE &&
+    hue < OWN_HUE + OWN_HUE_BLOCKED_SIZE
+  ) {
+    hue = hue - OWN_HUE // `hue` now at 0 +/- `OWN_HUE_BLOCKED_SIZE`
+    hue = hue + TOTAL_HUES - OWN_HUE_BLOCKED_SIZE
+  }
+
+  return hue
+}
+
+function getHueForId(id) {
+  const hash = md5(id)
+  const hue =
+    parseInt(hash.toString().slice(0, 8), 16) %
+    (TOTAL_HUES - OWN_HUE_BLOCKED_SIZE * 2)
+  return hue
+}

services/web/test/frontend/shared/utils/colors.test.js

@@ -0,0 +1,33 @@
+import { expect } from 'chai'
+
+import { getHueForUserId } from '../../../../frontend/js/shared/utils/colors'
+
+describe('colors', function () {
+  const currentUser = '5bf7dab7a18b0b7a1cf6738c'
+
+  describe('getHueForUserId', function () {
+    it('returns the OWN_HUE for the current user', function () {
+      expect(getHueForUserId(currentUser, currentUser)).to.equal(200)
+    })
+
+    it('returns the ANONYMOUS_HUE for an anonymous user', function () {
+      expect(getHueForUserId()).to.equal(100)
+      expect(getHueForUserId('anonymous-user')).to.equal(100)
+    })
+
+    it('generates a hue based on user id', function () {
+      expect(getHueForUserId('59ad79f46337430b3d37cb9e', currentUser)).to.equal(
+        146
+      )
+    })
+
+    it('shifts the hue away from the OWN_HUE if it is within a threshold', function () {
+      // Ordinarily, this user id would generate a hue of 183. However, this is
+      // visually "too close" to the OWN_HUE, meaning that it could be
+      // misinterpreted. Therefore we shift it away
+      expect(getHueForUserId('20ad79f46337430b3d37cb9f', currentUser)).to.equal(
+        323
+      )
+    })
+  })
+})