Feature/set agent color #325

interim17 · 2023-08-17T20:58:41Z

Problem

Closes Set Agent Color #148

Solution

In brief: colorChanges are now part of SelectionStateInfo. Viewer watches for changes to selectionStateInfo.colorChanges and calls changeAgentsColor in response to changes. changeAgentsColor updates visGeometry and UiDisplayData to ensure color changes are represented both in the viewport and in embedding applications that reference the UiDisplayData.

Added a few helper functions, cleaned unused ColorMapping in visGeo and made small updates to existing functions to accommodate new behavior.

Selection Interface:

Added property to the SelectionEntry type so that it now includes colorChanges
previous function setAgentColors divided into setInitialAgentColors and updateAgentColors

Viewport:

ComponentDidUpdate checks for new colorChangeAgents and calls changeAgentsColor which is the primary method for this PR

VisGeometry:

add applyColorToAgents and indexOfColor, remove unused ColorMapping functions

Example Viewer/ColorPicker.tsx

Added a new component file to prevent further growth of massive Viewer.tsx file, maybe a refactor of Viewer.tsx into different components would be wise at some point

New feature (non-breaking change which adds functionality)

Steps to Verify:

Try it out in the example viewer: select agents (and optionally subagents), select a color, apply color to agent. New colors can be provided via input field.

merge main

github-actions · 2023-08-17T21:01:36Z

jest coverage report 🧪

Total coverage

Status	Category	Percentage	Covered / Total
🔴	Statements	40.15%	1959/4879
🔴	Branches	43.82%	819/1869
🔴	Functions	37.13%	404/1088
🔴	Lines	40.42%	1877/4643

Status of coverage: 🟢 - ok, 🟡 - slightly more than threshold, 🔴 - under the threshold

Show files with reduced coverage 🔻

Reduced coverage

Status	Filename	Statements	Branches	Functions	Lines
🔴	index.ts	7.41%	7.31%	5.34%	7.4%
🔴	src/viewport	14.76%	12.3%	15.38% (-1.28% 🔻)	14.28%
🔴	index.tsx	14.76%	12.3%	15.38% (-1.28% 🔻)	14.28%
🔴	src/visGeometry	19.89%	21.73%	25.19%	19.9%

Status of coverage: 🟢 - ok, 🟡 - slightly more than threshold, 🔴 - under the threshold

src/visGeometry/index.ts

Feature/set agent color edits

toloudis · 2023-09-25T23:03:58Z

src/visGeometry/index.ts

        const colorNumber = convertColorStringToNumber(color);
        const newColor = [
            ((colorNumber & 0x00ff0000) >> 16) / 255.0,
            ((colorNumber & 0x0000ff00) >> 8) / 255.0,
            ((colorNumber & 0x000000ff) >> 0) / 255.0,
            1.0,
        ];
+        const currentIndex = this.indexOfColor(newColor);


when does addNewColor get called? Do you always know the agent type when you are calling it? ... trying to understand why indexOfColor is needed.

it's just in the case that the user changed the color of an agent to a color already in the data array

These get called in changeAgentsColor in viewport, which takes agent, tags, and color in the form of colorChanges. The function call that needs the IDs returned from addNewColor is setColorForIds in visGeo.

toloudis · 2023-09-25T23:04:56Z

src/simularium/SelectionInterface.ts

 import { EncodedTypeMapping } from "./types";
 import { convertColorNumberToString } from "../visGeometry/color-utils";
+import _ from "lodash";


can remove (see npm run lint output)

oops, fixed this and the two other things the linter caught

toloudis · 2023-09-25T23:10:33Z

src/simularium/SelectionInterface.ts

@@ -15,9 +18,15 @@ export interface SelectionEntry {
    tags: string[];
 }

+export interface ColorChanges {
+    agents: SelectionEntry[];


this would be better named as agentTypes.

on second thought, I realize we use "agents" in other SelectionEntry names as well. so at least it's consistent. Slightly confusing because we don't set colors on individual agents.

toloudis

LGTM!

meganrm · 2023-10-03T17:26:41Z

src/simularium/SelectionInterface.ts

+    ): void {
+        const newColor = convertColorNumberToString(color);
+        const entry = this.entries[agentName];
+        // if no display state update parent color


This comment isn't necessary

meganrm

looks good, there's just one comment I left to myself that is no longer relevant

meganrm · 2023-10-03T22:11:48Z

todo: make sure ColorChanges and SelectionEntry are exported from type-declarations before you merge this

ShrimpCryptid · 2023-10-05T15:24:29Z

examples/ColorPicker.tsx

+    uiDisplayData,
+    particleTypeNames,
+    agentColors,
+    setColorSelectionInfo,
+    updateAgentColorArray,


Just peeking in, should these props have type annotations or does it not need them?

I am guessing that the type checking rules are not set up to enforce this in our typecheck/linter. It would be good to turn those rules on in a separate PR!

The example Viewer code can be a bit fast and loose compared the to the core Viewer code. I like the idea of typing it better, or even refactoring it into component files someday when there is bandwidth.

I didn't realize this was the viewer test-app code. Totally agree that the bar is lower for those source files, although it doesn't need to be.

interim17 added 14 commits August 3, 2023 11:43

handle colorChangeAgents in selectioninterface

08382d4

apply color changes in viewport

531897e

set color change in visGeometry

37700b7

implement ui for color change in example viewer

9eb0c44

main function setAgentColor and helper functions

f388190

tighten up variable names

1479b45

clean up, rename functions

25c37a8

rename custom and assigned color props

ee6ed4a

add user colors to agentcolors

63522b6

update tests, add test for getColorChangeIds

3965da3

setColors function

4e13fb4

Merge branch 'main' into feature/set-agent-color

38962e0

merge main

add tag/subagent selection to example viewer

135ee7b

tidy example viewer

3d61f18

interim17 added 2 commits August 17, 2023 14:43

tidying viewer

04ba2d5

comment out colorMappingSet error

38b6fb1

interim17 commented Aug 17, 2023

View reviewed changes

src/visGeometry/index.ts Outdated Show resolved Hide resolved

meganrm and others added 12 commits August 23, 2023 15:42

move color picker out of viewer

29efd27

allow to add agent color

95cc6f7

move more of the color change to visGeometry

ccc229f

remove unneeded function

741e934

update tests

e0f0d88

Merge pull request #328 from simularium/feature/set-agent-color-edits

f6eae93

Feature/set agent color edits

update colorChanges typing to array

9c502b1

bug fix restore code to onTrajFileInfo

6fa78d2

cleanup unused colormapping in VisGeo

7d33f92

working updateUiData

81ebadd

send updated uidata to handler

498752d

clean up comments

16b162a

interim17 added 4 commits September 21, 2023 14:35

fix example viewer color change

73ad0f5

remove unused function

1531bf2

update tests

33afb92

adjust tests to code changes

8ddf9d2

interim17 marked this pull request as ready for review September 22, 2023 22:48

interim17 requested a review from a team as a code owner September 22, 2023 22:48

interim17 requested review from ShrimpCryptid, frasercl and ascibisz and removed request for a team September 22, 2023 22:48

toloudis reviewed Sep 25, 2023

View reviewed changes

unused import and const -> let

72c38cd

toloudis approved these changes Sep 26, 2023

View reviewed changes

meganrm reviewed Oct 3, 2023

View reviewed changes

meganrm approved these changes Oct 3, 2023

View reviewed changes

ShrimpCryptid reviewed Oct 5, 2023

View reviewed changes

interim17 added 3 commits October 5, 2023 12:17

typedeclarations colorchange and selectionentry

a36f698

export types from index

71649d0

remove unnecessary type export

ee7b583

interim17 merged commit 18e0547 into main Oct 5, 2023
5 of 6 checks passed

interim17 deleted the feature/set-agent-color branch October 5, 2023 22:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/set agent color #325

Feature/set agent color #325

interim17 commented Aug 17, 2023 •

edited

Loading

github-actions bot commented Aug 17, 2023 •

edited

Loading

Reduced coverage

toloudis Sep 25, 2023 •

edited

Loading

meganrm Sep 26, 2023

interim17 Sep 26, 2023

toloudis Sep 25, 2023 •

edited

Loading

interim17 Sep 25, 2023

toloudis Sep 25, 2023

toloudis Sep 25, 2023

toloudis left a comment

meganrm Oct 3, 2023 •

edited

Loading

meganrm left a comment

meganrm commented Oct 3, 2023 •

edited

Loading

ShrimpCryptid Oct 5, 2023 •

edited

Loading

toloudis Oct 5, 2023

interim17 Oct 5, 2023

toloudis Oct 5, 2023

Feature/set agent color #325

Feature/set agent color #325

Conversation

interim17 commented Aug 17, 2023 • edited Loading

Problem

Solution

Steps to Verify:

github-actions bot commented Aug 17, 2023 • edited Loading

jest coverage report 🧪

Total coverage

Reduced coverage

toloudis Sep 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toloudis Sep 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toloudis left a comment

Choose a reason for hiding this comment

meganrm Oct 3, 2023 • edited Loading

Choose a reason for hiding this comment

meganrm left a comment

Choose a reason for hiding this comment

meganrm commented Oct 3, 2023 • edited Loading

ShrimpCryptid Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

interim17 commented Aug 17, 2023 •

edited

Loading

github-actions bot commented Aug 17, 2023 •

edited

Loading

toloudis Sep 25, 2023 •

edited

Loading

toloudis Sep 25, 2023 •

edited

Loading

meganrm Oct 3, 2023 •

edited

Loading

meganrm commented Oct 3, 2023 •

edited

Loading

ShrimpCryptid Oct 5, 2023 •

edited

Loading