Skip to content

GitHub Issue 916, 917, 932, 942 & 961#1955

Open
XingY wants to merge 7 commits intodevelopfrom
fb_mvtc_bash2
Open

GitHub Issue 916, 917, 932, 942 & 961#1955
XingY wants to merge 7 commits intodevelopfrom
fb_mvtc_bash2

Conversation

@XingY
Copy link
Contributor

@XingY XingY commented Mar 16, 2026

Rationale

Updated insertPastedData to be able to handle comma better for the following issues.

  • GitHub Issue 916: Copying/pasting in the grid doesn't always act as expected
  • GitHub Issue 917: Copying/pasting in the grid doesn't reorder selection
  • GitHub Issue 942: Add error for duplicate values for MVTC fields

Some UX update for "Allow multiple selections" checkbox

  • GitHub Issue 961: Clicking the "Allow multiple selections" label doesn't toggle the checkbox
  • GitHub Issue 932: No help text for disabled Multi-Value checkbox in designer

Related Pull Requests

Changes

  • Updated parseCsvString to be tolerant of whitespace before double quote to handle parsing of A, "B,C" vs A,"B,C"
  • Added a check for isSingleColPaste in insertPastedData to prioritize exact single match without parsing
  • Added check for duplicate values for multi-choice fields

while (start < value.length) {
let end;
const ch = value[start];
// Tolerate a single space before a properly quoted value
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this handle space after the quotes as well?

expect(parseCsvString('1, "2,3" , 4', ',', true)).toStrictEqual(['1', '2,3', ' 4']);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't feel strong about this one but I could be convinced otherwise. I also didn't handle when there are multiple white spaces. BTW, I tagged you for re-review since I pushed additional changes for 2 small designer UI issues. I don't plan to include more issues in this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Alright, perhaps put a TODO comment here regarding support for multiple spaces and spaces after quoted values.

@XingY XingY changed the title GitHub Issue 942 & 916 GitHub Issue 916, 932, 942 & 961 Mar 17, 2026
@XingY XingY requested a review from labkey-nicka March 17, 2026 17:01
@XingY XingY changed the title GitHub Issue 916, 932, 942 & 961 GitHub Issue 916, 917, 932, 942 & 961 Mar 18, 2026
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