Skip to content
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.

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

[Vue] provide change abort functionality for Fields via model modifiers #19513

Merged
merged 2 commits into from Jul 21, 2022

Conversation

diosmosis
Copy link
Member

@diosmosis diosmosis commented Jul 11, 2022

Description:

Fixes #19509

Original problem: some plugins need to prevent an input from changing, like Funnels which needs to prevent a checkbox from changing until the user deals w/ a modal, but Vue will not notice when a model value doesn't change, and won't reset the input value in the case the parent component doesn't update the model value.

Fixes that didn't work:

  • resetting the input value before emitting update:modelValue. This reset the input state to what the modelValue was in case the parent didn't update the modelValue, but changing the input's value reset the text cursor position which makes editing the middle of an input/textarea very annoying.
  • resetting the input after emitting update:modelValue. This ends up always resetting the value because there's no way to know when the emitted event is done being handled, causing a consistent flash/backspace when typing.

This solution (only other one I could come up with):

This PR adds an 'abortable' model modifier that can be used to specify a Field can have it's change aborted. In this case, the event sent to update:modelValue has an abort() function that will reset the input value. This way the input value is only reset if the event handler aborts it, so the text cursor position never changes and there is no weird flash/backspace when typing. It can't really be used like a normal model modifier (ie, v-model.abortable) since it changes the event value, but this functionality is used very rarely in Matomo so I don't think that's much of an issue.

Usage is like:

<Field :uicontrol="text" :model-value="value" :model-modifiers="{abortable:true}" @update:model-value="doUpdate($event)"/>

doUpdate(event: AbortableEvent<string>) {
    if (...) {
        event.abort();
        return;
    }

    this.value = event.value;
}

Review

@diosmosis diosmosis added this to the 4.12.0 milestone Jul 11, 2022
Copy link
Contributor

@peterhashair peterhashair left a comment

Choose a reason for hiding this comment

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

test UI working as expected

@sgiehl
Copy link
Member

sgiehl commented Jul 20, 2022

@diosmosis could you maybe send me a link to a PR where that abort modifier is being used? I would like to see that somewhere in action 🙈

@diosmosis
Copy link
Member Author

diosmosis commented Jul 21, 2022

@sgiehl you can look in pull request 93 in Funnels. The last commit adds it. I tested locally before pushing.

@sgiehl
Copy link
Member

sgiehl commented Jul 21, 2022

@diosmosis Even though the Funnel plugins seems to require the usage of the abort event in some other cases, it seems to work for the one case where you have implemented it. 👍

@sgiehl sgiehl merged commit ca1b52a into next_release Jul 21, 2022
@sgiehl sgiehl deleted the vue-abortable-field branch July 21, 2022 08:45
@sgiehl sgiehl modified the milestones: 4.12.0, 4.11.0 Jul 21, 2022
@diosmosis
Copy link
Member Author

@sgiehl can you let me know on the Funnels pr which places so I can add them there?

@justinvelluppillai justinvelluppillai added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants