Skip to content

Sample type color field and color configuration - #2038

Open
XingY wants to merge 16 commits into
developfrom
fb_sampleColors
Open

Sample type color field and color configuration#2038
XingY wants to merge 16 commits into
developfrom
fb_sampleColors

Conversation

@XingY

@XingY XingY commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Rationale

Adds the components and actions for configuring sample colors.

Related Pull Requests

Changes

  • New App.isSampleColorsEnabled that checks experimental flag
  • New ManageSampleColorsPanel that support add/edit/archive/delete colors and enable/disable sample types for each color
  • New SampleColorsSetting to allow managing sample type colors on designer
  • SampleColorRenderer/SampleColorInput: render sample colors (to be wired up on story 2), and support filtered color inputs based on sample type active colors
  • new utils for querying/managing colors and sample type colors

@XingY
XingY requested a review from cnathe July 22, 2026 22:17
Comment thread packages/components/src/theme/form.scss Outdated
sampleTypeName?: string,
containerPath?: string
): Promise<number[]> {
const rowId = sampleTypeRowId ?? (await getSampleTypeRowId(sampleTypeName));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since both sampleTypeRowId and sampleTypeName are optional params, should we have a quick check that one of them is provided here and error early if not?

return new Promise((resolve, reject) => {
selectRows({
schemaQuery: SCHEMAS.EXP_TABLES.SAMPLE_SETS,
columns: 'Name,' + fieldKey,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this was already like this before you brought the code over from the premium package, but any idea why there isn't a filter here on the name? instead it queries for all sample types and then does the "filter" in the response. That seems inefficient.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this has to do with case sensitivity? There might be case where sampletype name is parsed from URL, and could have a different casing?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yes, that is probably why

Comment thread packages/components/src/internal/components/samples/ManageSampleColorsPanel.tsx Outdated
}, [updated, excludedTypes, initialExcludedTypes, saveColor]);

const onToggleArchive = useCallback(() => {
saveColor({ ...updated, archived: !updated.archived });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in manual testing, you can select a color > uncheck some sample types to make them excluded > click archive. it makes it seem like that color has the exclusions but they were not saved with the saveColor here. This seems like an edge case as people are unlikely to try to set exclusions while archiving.

Comment thread packages/components/src/internal/components/samples/ManageSampleColorsPanel.tsx Outdated
Comment thread packages/components/src/internal/components/samples/SampleColorsSetting.tsx Outdated
Comment thread packages/components/src/internal/components/samples/SampleColorsSetting.tsx Outdated
Comment thread packages/components/src/internal/components/samples/SampleColorsSetting.tsx Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants