color chooser feedback

[tmazor]

Overall the functionality looks pretty good. Some thoughts in no particular order:

1. The selected color is shown as a circle colored border with white fill -- but that makes me think that the circle was showing the option to select white as a color. Can the selected color keep the colored fill but maybe add a thick black border to indicate the selection?
   

2. What determines if a group comes with a pre-assigned color vs doesn't? I created groups based on Sex or Genomic Profile availability:
   
   Male & Female have colors, and that I can understand since we define default colors for those values. But why does 'fusion' come with a color when 'mutations' and 'log2CNA' don't ?

3. I'm struggling with the symbol that appears for groups that don't have a defined color. The symbol (square with red diagonal line) makes me think of some sort of null value that needs to be fixed - in the screenshot above, the fact that 2 groups have that square with the red line and the other 3 groups have colors makes me think I have to assign colors to all groups or else they may not work.

Is there another symbol that would be more intuitive for 'you can select a color here but you don't have to' ? At the very least, perhaps updating the tooltip to say something like 'Optional: Select color for group to be used in group comparison. If no color is selected, a random color will be applied.'

4. When groups are shared, they are shared without their assigned colors:
   
   https://www.cbioportal.org/study/summary?id=gbm_tcga_pan_can_atlas_2018#sharedGroups=6081a1d3e4b015b63e9e78d0,6081a1e0e4b015b63e9e78d1,6081a1ece4b0242bd5d49ccf,6081a1ffe4b015b63e9e78d2,6081a20be4b0242bd5d49cd0,6081a42ae4b0242bd5d49cdc
   Is this intentional? I expected colors to come with the groups.

5. Why is the groups dropdown refreshing every time I select a group?
   

[dianab0]

Hi Tali,

Please see my answers below:

1. We have used the Circle Picker from react-color and this is how it looks like. There is no option on changing design for the selected circle. https://casesandberg.github.io/react-color/

2. There is the ./src/shared/lib/Colors.ts which defines the reserved colors. Fusion is in this list while log2CNA and mutations are not.
   export let RESERVED_CLINICAL_VALUE_COLORS: { [value: string]: string } = {
   ...
   missense: MUT_COLOR_MISSENSE,
   inframe: MUT_COLOR_INFRAME,
   truncating: MUT_COLOR_TRUNC,
   splice: MUT_COLOR_SPLICE,
   fusion: MUT_COLOR_FUSION,
   promoter: MUT_COLOR_PROMOTER,
   driver: MUT_COLOR_MISSENSE,
   vus: MUT_COLOR_MISSENSE_PASSENGER,
   ...
   }

3. I have updated the tooltip message and pushed the change on top of Rfc55 Color Chooser cbioportal-frontend#3710. I think this message is indeed more clear about how this works. No idea on different empty color icon.

4. It is intentional and I remember there was a discussion about this.
   The color is per user, saved in the user session and should not be shared to other users. Groups must also be immutable, so if a user is changing the color, it should not change for all other users the group was shared with.

5. This seems to be an old behavior - I have checked cbioportal without color chooser code and it is still happening.
   So in both cases, with color chooser code and without, the groups dropdown is refreshing when selecting groups only the first time after a page refresh. After closing and reopening the group dropdown it is no longer happening.
   This is something that should be investigated but wasn't cause by color chooser.

[tmazor]

Sorry for the delay @dianab0 - see my responses below:

1. Can we use a different one of those? I think the color chooser labeled 'Github' is much clearer (or Twitter if you can remove the hex code)
2. Oh so those colors are defined for OncoPrint, but it's all stored in a shared place so it's also getting applied here? I still think it's confusing but maybe beyond the scope of this.
3. Looks good
4. I agree that if a user changes colors for a shared group, that the color should not change for other users. But I think that when a group is initially shared, it should be shared with the color. From my perspective, if I've created groups, and gone the extra step of assigning colors for those groups, and then want to share those groups, I expect the colors will be part of what is shared. What was the reason for not wanting to share color?
5. I will make a separate ticket for this

[dianab0]

Hi @tmazor

1. I have changed it to Github color chooser and will make a pull request for it. Right not it looks like this:
   
   
2. Yes, correct.
3. The decision to not share the color and save the color group into the user session instead of the group session, because groups must remain immutable, was made by MSK during a release/planning meeting.
   My initial implementation was to save the group color into the group session (so the sharing of the color worked), but after this decision we had to do a refactoring and change where the color of a group is saved. So now with the color saved in the user session, it is quite impossible to share it anymore, and by this it would also mean that a user is attempting to change other user’s session. Also it might generate some confusion if for example the user shares the group to user1 with a color, and then he changes the color of his group and shares it again to user2. So multiple users might end up having different color in their user session that wasn’t set by themselves.

[dianab0]

Hi @tmazor

There is an issue related to the github picker. After choosing a color and closing the github picker, when opening it again the active color is not shown (neither highlighted nor selected). This is a bug in the library, which I fixed and created a PR for it: casesandberg/react-color#824
I'll wait for a response/review on this fix and meanwhile will keep the circle picker in cbioportal.

Below are 2 gifs with the behavior of github picker in cbioportal before and after my fix.
Before:
After: