From fefb588d71160be2568a6f628efca3ad68352c00 Mon Sep 17 00:00:00 2001 From: Jessica Lawshe Date: Tue, 18 May 2021 08:43:36 -0500 Subject: [PATCH] Merge pull request #3973 from overleaf/as-clean-up-hue-generator Clean up hue generator GitOrigin-RevId: 1b7fad05d7c2f6519efbbe0379f2449c273e4f4b --- .../js/features/chat/components/message.js | 4 +- .../components/online-users-widget.js | 6 +-- .../frontend/js/ide/colors/ColorManager.js | 3 ++ .../web/frontend/js/shared/utils/colors.js | 38 +++++++++++++++++++ .../test/frontend/shared/utils/colors.test.js | 33 ++++++++++++++++ 5 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 services/web/frontend/js/shared/utils/colors.js create mode 100644 services/web/test/frontend/shared/utils/colors.test.js diff --git a/services/web/frontend/js/features/chat/components/message.js b/services/web/frontend/js/features/chat/components/message.js index e75e5c546b..9b84a45e0d 100644 --- a/services/web/frontend/js/features/chat/components/message.js +++ b/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) { diff --git a/services/web/frontend/js/features/editor-navigation-toolbar/components/online-users-widget.js b/services/web/frontend/js/features/editor-navigation-toolbar/components/online-users-widget.js index 3b01c1c93c..2f9c3fde47 100644 --- a/services/web/frontend/js/features/editor-navigation-toolbar/components/online-users-widget.js +++ b/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) diff --git a/services/web/frontend/js/ide/colors/ColorManager.js b/services/web/frontend/js/ide/colors/ColorManager.js index 78a5a6545d..39ca78454d 100644 --- a/services/web/frontend/js/ide/colors/ColorManager.js +++ b/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 diff --git a/services/web/frontend/js/shared/utils/colors.js b/services/web/frontend/js/shared/utils/colors.js new file mode 100644 index 0000000000..4a3434597c --- /dev/null +++ b/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 +} diff --git a/services/web/test/frontend/shared/utils/colors.test.js b/services/web/test/frontend/shared/utils/colors.test.js new file mode 100644 index 0000000000..bf952dae79 --- /dev/null +++ b/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 + ) + }) + }) +})