Skip to content

fix: measure filter apply #9319

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Closed
royendo wants to merge 5 commits into from
+25 −27
Closed

fix: measure filter apply #9319

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f9b0501
fix: measure filter `Apply` button not submitting in Svelte 5
royendo Apr 28, 2026
10ac1c4
fix: set `dataType: "json"` in measure filter superForm
royendo Apr 28, 2026
9d6bc8a
fix: bypass superforms submit chain in `MeasureFilterForm`
royendo Apr 28, 2026
1789577
debug: add native button + Apply onClick log to diagnose click flow
royendo Apr 28, 2026
1151d70
refactor: simplify `MeasureFilterForm` apply flow
royendo Apr 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 25 additions & 27 deletions web-common/src/features/dashboards/filters/measure-filters/MeasureFilterForm.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,34 +60,37 @@
}),
});

const { form, errors, submit, enhance } = superForm(
const { form, errors, validateForm } = superForm(
defaults(initialValues, yup(validationSchema)),
{
SPA: true,
validators: yup(validationSchema),
onUpdate({ form }) {
if (!form.valid) return;
const values = form.data;

onApply({
dimension: values.dimension,
oldDimension: dimensionName,
filter: {
measure: name,
operation: values.operation,
type: MeasureFilterType.Value,
value1: values.value1,
value2: values.value2 ?? "",
},
});

open = false;
},
dataType: "json",
invalidateAll: false,
resetForm: false,
},
);

async function submit() {
Copy link
Copy Markdown
Collaborator

@AdityaHegde AdityaHegde Apr 28, 2026

Choose a reason for hiding this comment

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

Do we need this? Cant we just use the old onUpdate with dataType: "json"?

Copy link
Copy Markdown
Contributor Author

@royendo royendo Apr 28, 2026

Choose a reason for hiding this comment

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

testing without

Copy link
Copy Markdown
Contributor Author

@royendo royendo Apr 28, 2026

Choose a reason for hiding this comment

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

yeah idk when i revert to 10ac1c4, it doesnt close the modal and apply; tried a few variations and the one i landed on works;

lmk if you want to take over a look at it properly

Copy link
Copy Markdown
Collaborator

@AdityaHegde AdityaHegde Apr 28, 2026

Choose a reason for hiding this comment

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

This is because of nested forms with measure filter dropdown. Making sure measure filter form submits correctly instead here: #9322

That way it will also work with reports.

const result = await validateForm({ update: true });
if (!result.valid) return;
const values = result.data;

onApply({
dimension: values.dimension,
oldDimension: dimensionName,
filter: {
measure: name,
operation: values.operation,
type: MeasureFilterType.Value,
value1: values.value1,
value2: values.value2 ?? "",
},
});

open = false;
}

$: ({ operation } = $form);

$: isBetweenExpression = expressionIsBetween(operation);
Expand Down Expand Up @@ -141,12 +144,7 @@
/>
</div>
{/if}
<form
use:enhance
autocomplete="off"
class="flex flex-col gap-y-3"
id="measure"
>
<div class="flex flex-col gap-y-3">
<Select
bind:value={$form["dimension"]}
id="dimension"
Expand Down Expand Up @@ -193,6 +191,6 @@
/>
{/if}

<Button submitForm type="primary" form="measure">Apply</Button>
</form>
<Button type="primary" onClick={submit}>Apply</Button>
</div>
</Popover.Content>
Loading