Skip to content

Conversation

@XingY
Copy link
Contributor

@XingY XingY commented Jan 12, 2026

Rationale

This PR enables "Multi Choice" data type in domain designer and supports editing/sorting multi values in the app.

Related Pull Requests

Changes

  • Added new MULTI_CHOICE_RANGE_URI for defining MVTC fields and updated utils to check against data type
  • renamed joinValues prop for SelectInput to skipJoinValues to align with its actual usage
  • Modified FilterFacetedSelector and QueryFilterPanel to handle array value selecting for MVTC

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

This comment was marked as resolved.

@XingY XingY requested a review from labkey-alan January 14, 2026 21:28
Comment on lines 816 to 819
if (
(type === 'TextChoice' || type === 'MultiChoice') &&
!rawPropertyValidator[i]?.properties?.validValues
) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would define a variable isChoice below the hasExpressionStr variable above:

const isChoice = type === 'TextChoice' || type === 'MultiChoice';

Then the if statement will fit on one line instead of 4, and be a little easier to read.

Suggested change
if (
(type === 'TextChoice' || type === 'MultiChoice') &&
!rawPropertyValidator[i]?.properties?.validValues
) {
if (isChoice && !rawPropertyValidator[i]?.properties?.validValues) {

@XingY XingY requested a review from labkey-alan January 16, 2026 02:07
Copy link
Contributor

@labkey-alan labkey-alan left a comment

Choose a reason for hiding this comment

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

Left some minor feedback but otherwise looks good.

api?: ComponentsAPIWrapper;
canBeBlank: boolean;
disabled?: boolean;
field?: QueryColumn;
Copy link
Contributor

Choose a reason for hiding this comment

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

field is always passed, so it should be defined as: field: QueryColumn. If it could be undefined then the type should be field: QueryColumn | undefined.

Suggested change
field?: QueryColumn;
field: QueryColumn;

fieldFilters: Filter.IFilter[];
fieldKey: string;
onFieldFilterUpdate?: (newFilters: Filter.IFilter[], index) => void;
onFieldFilterUpdate?: (newFilter: Filter.IFilter[]) => void;
Copy link
Contributor

Choose a reason for hiding this comment

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

onFieldFilterUpdate is always passed so it should be defined as:

Suggested change
onFieldFilterUpdate?: (newFilter: Filter.IFilter[]) => void;
onFieldFilterUpdate: (newFilter: Filter.IFilter[]) => void;

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.

3 participants