[stable31] fix: adjust to workflowengine changes and register web component #311
Conversation
blizzz
added
2. developing
Signed-off-by: Arthur Schiwon <[email protected]>
blizzz
force-pushed
the
backport/308/stable31
branch
2 times, most recently
from
1ebc092 to
39fd040
Compare
This comment was marked as resolved.
This comment was marked as resolved.
blizzz
force-pushed
the
backport/308/stable31
branch
from
39fd040 to
b5f8edd
Compare
|
/backport to stable30 |
backportbot
bot
added
the
backport-request
blizzz
added
3. to review
|
|
||
| let customElementId | ||
| // we may only compare first three numbers of the version | ||
| if (semver.gt(window.OC.config.version.split('.', 3).join('.'), '31.0.2')) { |
There was a problem hiding this comment.
Without this condition, I get this.$root.props is undefined on NC 31 <= 31.0.2 even though the component is not used 😒
otoh there is also no necessity to create it for nothing, so it is not bad to have the check there.
There was a problem hiding this comment.
I briefly checked for related API changes that we could detect instead but found none.
Not sure which line throws the undefined error - we could also catch that. But this seems most explicit and clean anyway.
There was a problem hiding this comment.
This only appears when the component is actually not used. Somewhere along the store's addRule call (i showed you the strace). My explanation for now is that there is some event (handler) or so being called and since this is not embedded properly anywhere it stumbles.
max-nextcloud
left a comment
max-nextcloud
left a comment
There was a problem hiding this comment.
Just a bit of nitpicking about complex input handling. Other than that the code looks good.
There was a problem hiding this comment.
To be frank I don't quite understand what's going on here. It seems overly complicated but I don't exactly know what you are trying to achieve. I will start from top to bottom and add a few comments to write down (and maybe improve) my understanding.
src/views/FlowNotify.vue
Outdated
| if (value === null) { | ||
| return | ||
| } | ||
| this.inscription = value.target.value |
There was a problem hiding this comment.
Yes... this should not be necessary as v-model should take care of it. Maybe the emitInput function is triggered before the v-model sync.
I'd suggest moving this to a watch on inscription then you can be sure the value was already updated. This would also save you the check for value === null (where I'm also unsure if it's needed).
There was a problem hiding this comment.
Yeah, totally right. Interesting too see that vue let's you solve things in different ways. This one wasn't suggested to me before 😓 but I followed and replaced this event handler with the watcher.
|
Thank you for your input @max-nextcloud I'll try to reduce it further, checking against both 30.0.2 and stable31. |
blizzz
added
2. developing
Signed-off-by: Arthur Schiwon <[email protected]>
blizzz
force-pushed
the
backport/308/stable31
branch
from
b5f8edd to
28a5def
Compare
blizzz
added
3. to review
backportbot
bot
removed
the
backport-request
Backport of PR #308