Skip to content
This repository has been archived by the owner on Jan 3, 2024. It is now read-only.

Fixed prop mutation, wrong event and confirm button protection in the modal #741

Merged
merged 1 commit into from
May 13, 2020

Conversation

LukasHirt
Copy link
Collaborator

@LukasHirt LukasHirt commented May 4, 2020

Properly handle input value via v-model, use input event and disable confirm button in case of an error.

@LukasHirt LukasHirt self-assigned this May 4, 2020
@LukasHirt LukasHirt force-pushed the bugfix/modal-v-model branch from 49e08f1 to b89c580 Compare May 12, 2020 10:42
@LukasHirt LukasHirt marked this pull request as ready for review May 12, 2020 10:42
@LukasHirt LukasHirt requested review from kulmann and PVince81 May 12, 2020 10:42
src/elements/OcModal.vue Outdated Show resolved Hide resolved
src/elements/OcModal.vue Outdated Show resolved Hide resolved
Copy link
Contributor

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

Aaah, the intention is, to emit a type event on value changes coming from v-model, correct? In that case you are doing some extra steps that could be avoided.
One more thing: I don't see an initializer for $_ocModal_input_value variable. You need to set that in the mounted lifecycle step, right?

src/elements/OcModal.vue Outdated Show resolved Hide resolved
src/elements/OcModal.vue Show resolved Hide resolved
@kulmann
Copy link
Contributor

kulmann commented May 12, 2020

Why not just emit input instead of type? I think the input event reflects properly what is happening. What do you think? 🤔

@LukasHirt
Copy link
Collaborator Author

One more thing: I don't see an initializer for $_ocModal_input_value variable. You need to set that in the mounted lifecycle step, right?

This is done via the watcher.

Why not just emit input instead of type? I think the input event reflects properly what is happening. What do you think? 🤔

Makes sense, will do 😉

@LukasHirt LukasHirt force-pushed the bugfix/modal-v-model branch from 43699b9 to d53f55e Compare May 13, 2020 07:55
@LukasHirt
Copy link
Collaborator Author

@kulmann Ready for next review 😉

Copy link
Contributor

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

nice 👍

src/elements/OcModal.vue Show resolved Hide resolved
@LukasHirt LukasHirt merged commit 7210dc9 into master May 13, 2020
@LukasHirt LukasHirt deleted the bugfix/modal-v-model branch May 13, 2020 08:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants