Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Improve validation of Plot Wizard Y metrics quick pick #4827

Merged
merged 3 commits into from
Oct 16, 2023

Conversation

julieg18
Copy link
Contributor

@julieg18 julieg18 commented Oct 13, 2023

  • If there are multiple x metrics, the quick pick has a different title and has a max number of items

Demo

Screen.Recording.2023-10-13.at.10.54.23.AM.mov

Followup to #4789 comment

@julieg18 julieg18 added the product PR that affects product label Oct 13, 2023
@julieg18 julieg18 self-assigned this Oct 13, 2023
return {
fields: { x, y },
firstXKey: (xValues as QuickPickFieldValues)[0].field,
firstYKey: (yValues as QuickPickFieldValues)[0].field
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Missed this earlier, we were using key instead of field.

{
title: `Select ${xFieldsLength} Metrics for Y` as Title
},
xFieldsLength
Copy link
Contributor Author

Choose a reason for hiding this comment

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

One issue with the quick pick is that we lose separators (aka file names) when the items are collapsed:

image

const y =
xValuesLength > 1
? await pickYFieldsWithMultiXFields(yItems, xValuesLength)
: await pickYFields(yItems)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With this change, y fields are picked and validated differently depending on if there are multiple x values.

@julieg18 julieg18 marked this pull request as ready for review October 13, 2023 16:23
@julieg18 julieg18 enabled auto-merge (squash) October 16, 2023 12:47
@codeclimate
Copy link

codeclimate bot commented Oct 16, 2023

Code Climate has analyzed commit 5c43e0d and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

The test coverage on the diff in this pull request is 95.0% (85% is the threshold).

This pull request will bring the total coverage in the repository to 95.0% (0.0% change).

View more on Code Climate.

@julieg18 julieg18 merged commit 215cad7 into main Oct 16, 2023
@julieg18 julieg18 deleted the improve-plot-wizard-y-metrics-quick-pick branch October 16, 2023 13:11
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
product PR that affects product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants