VMO-3852 Support deep linking for simulator + VMO-3687 Simulator on read only view#75
VMO-3852 Support deep linking for simulator + VMO-3687 Simulator on read only view#75poojahpatil90 wants to merge 10 commits intomasterfrom
Conversation
…ator # Conflicts: # builder.config.json # src/components/interaction-designer/toolbar/TreeBuilderToolbar.vue # src/store/flow/block-types/MobilePrimitives_SelectManyResponseBlockStore.ts # src/views/InteractionDesigner.vue # tests/unit/__snapshots__/storybook.spec.ts.snap
…-simulator # Conflicts: # src/components/interaction-designer/toolbar/TreeBuilderToolbar.vue # src/store/clipboard/index.ts # src/views/InteractionDesigner.vue # tests/unit/__snapshots__/storybook.spec.ts.snap
| }) | ||
| return { | ||
| params: { | ||
| id: this.activeFlow?.uuid, |
There was a problem hiding this comment.
Wouldn't this be treeId: this.activeFlow?.uuid ?
There was a problem hiding this comment.
And I'm a bit confused why we removed component: ?
There was a problem hiding this comment.
@safydy-r Good question! There are two ways to use routes:
- Use
route.from({component: 'interaction-designer', mode: 'view', treeId: this.activeFlow?.uuid})from routes.js mixin. Example: See here - Use Vue router's route name and pass the parameters as defined in the route config. Example: See here
Here the route is already setup beforehand and when we are click on Edit/View Flow button in the toolbar, I am just passing in the new mode along with flowId(just to be sure). I feel it is much simpler to use routes this way rather than having a dependency on builder.config and routes mixin.
Thoughts?
There was a problem hiding this comment.
@poojahpatil90
Sorry, I still don't understand, hihi 😸 . But it seems you know what you are doing here. I just love to have a quick call to understand why we still providing mode: this.isEditable ? 'view' : 'edit' here and not component: 'interaction-designer',.
| if (field) { | ||
| scrollBehavior(this.$route) | ||
| } | ||
| if (this.$route.name === 'flow-simulator' && this.hasSimulator()) { |
There was a problem hiding this comment.
As we already have this condition here, what if we change <div v-if="$route.name === 'flow-simulator' && hasSimulator" class="tree-sidebar"> from this line
https://github.com/FLOIP/flow-builder/blob/VMO-3852-deep-linking-simulator/src/views/InteractionDesigner.vue#L21 with <div v-if="isSimulatorActive" class="tree-sidebar">
Would that work?
There was a problem hiding this comment.
@safydy-r This condition has been added here to open up the simulator when the URL says /simulator. This is needed here specifically to handle UI refresh and the URL being pasted in a new tab/window of the browser.
In order to use isSimulatorActive, it needs to be set true already. but as you can see I am setting it below after catching the simulator URL.
I agree comparing the route name isn't ideal, but I cannot think of any better solution. Let me know your thoughts?
src/components/interaction-designer/toolbar/TreeBuilderToolbar.vue
Outdated
Show resolved
Hide resolved
safydy-r
left a comment
There was a problem hiding this comment.
Overall I'm ok, but left few questions for you @poojahpatil90
# Resolved Conflicts: # src/components/interaction-designer/toolbar/TreeBuilderToolbar.vue # src/views/InteractionDesigner.vue # tests/unit/__snapshots__/storybook.spec.ts.snap
| @@ -58,7 +64,7 @@ export const actions: ActionTree<IClipboardState, IRootState> = { | |||
| }, | |||
| removeFromBlocksData({ state }, index: number) { | |||
There was a problem hiding this comment.
Reporter: ESLint
Rule: eslint.rules.object-curly-spacing
Severity: ERROR
File: src/store/clipboard/index.ts L65
There should be no space after '{'. (object-curly-spacing)
| @@ -58,7 +64,7 @@ export const actions: ActionTree<IClipboardState, IRootState> = { | |||
| }, | |||
| removeFromBlocksData({ state }, index: number) { | |||
There was a problem hiding this comment.
Reporter: ESLint
Rule: eslint.rules.object-curly-spacing
Severity: ERROR
File: src/store/clipboard/index.ts L65
There should be no space before '}'. (object-curly-spacing)
src/store/clipboard/index.ts
Outdated
| export const actions: ActionTree<IClipboardState, IRootState> = { | ||
| setSimulatorActive({ commit }, value) { | ||
| commit('setSimulatorActive', value) | ||
| const routeName = value ? 'flow-simulator' : 'flow-details' |
There was a problem hiding this comment.
Reporter: ESLint
Rule: eslint.rules.@typescript-eslint/strict-boolean-expressions
Severity: ERROR
File: src/store/clipboard/index.ts L43
Unexpected any value in conditional. An explicit comparison or type cast is required. (@typescript-eslint/strict-boolean-expressions)
| @@ -38,6 +40,10 @@ export const mutations: MutationTree<IClipboardState> = { | |||
| export const actions: ActionTree<IClipboardState, IRootState> = { | |||
| setSimulatorActive({ commit }, value) { | |||
There was a problem hiding this comment.
Reporter: ESLint
Rule: eslint.rules.object-curly-spacing
Severity: ERROR
File: src/store/clipboard/index.ts L41
There should be no space after '{'. (object-curly-spacing)
| @@ -38,6 +40,10 @@ export const mutations: MutationTree<IClipboardState> = { | |||
| export const actions: ActionTree<IClipboardState, IRootState> = { | |||
| setSimulatorActive({ commit }, value) { | |||
There was a problem hiding this comment.
Reporter: ESLint
Rule: eslint.rules.object-curly-spacing
Severity: ERROR
File: src/store/clipboard/index.ts L41
There should be no space before '}'. (object-curly-spacing)
| blocksData: (state) => state.blocksData, | ||
| getBlockPrompt: (state) => (index: number) => state.blocksData[index].prompt, | ||
| isBlockFocused: (state) => (index: number) => state.blocksData[index].isFocused, | ||
| hasSimulator: (_, _2, _3, rootGetters) => rootGetters['flow/hasOfflineMode'] && rootGetters.isFeatureSimulatorEnabled && !rootGetters['builder/isEditable'], |
There was a problem hiding this comment.
Reporter: ESLint
Rule: eslint.rules.max-len
Severity: WARN
File: src/store/clipboard/index.ts L28
This line has a length of 158. Maximum allowed is 140. (max-len)
| blocksData: (state) => state.blocksData, | ||
| getBlockPrompt: (state) => (index: number) => state.blocksData[index].prompt, | ||
| isBlockFocused: (state) => (index: number) => state.blocksData[index].isFocused, | ||
| hasSimulator: (_, _2, _3, rootGetters) => rootGetters['flow/hasOfflineMode'] && rootGetters.isFeatureSimulatorEnabled && !rootGetters['builder/isEditable'], |
There was a problem hiding this comment.
Reporter: ESLint
Rule: eslint.rules.@typescript-eslint/strict-boolean-expressions
Severity: ERROR
File: src/store/clipboard/index.ts L28
Unexpected any value in conditional. An explicit comparison or type cast is required. (@typescript-eslint/strict-boolean-expressions)
| blocksData: (state) => state.blocksData, | ||
| getBlockPrompt: (state) => (index: number) => state.blocksData[index].prompt, | ||
| isBlockFocused: (state) => (index: number) => state.blocksData[index].isFocused, | ||
| hasSimulator: (_, _2, _3, rootGetters) => rootGetters['flow/hasOfflineMode'] && rootGetters.isFeatureSimulatorEnabled && !rootGetters['builder/isEditable'], |
There was a problem hiding this comment.
Reporter: ESLint
Rule: eslint.rules.@typescript-eslint/strict-boolean-expressions
Severity: ERROR
File: src/store/clipboard/index.ts L28
Unexpected any value in conditional. An explicit comparison or type cast is required. (@typescript-eslint/strict-boolean-expressions)
| import { IPrompt } from '@floip/flow-runner' | ||
|
|
||
| export interface BlocksData { | ||
| isFocused: boolean; |
There was a problem hiding this comment.
Reporter: ESLint
Rule: eslint.rules.@typescript-eslint/member-delimiter-style
Severity: WARN
File: src/store/clipboard/index.ts L9
Expected a comma. (@typescript-eslint/member-delimiter-style)
| import { IRootState } from '@/store' | ||
| import { IPrompt } from '@floip/flow-runner'; | ||
| import { router } from '@/router' | ||
| import { IPrompt } from '@floip/flow-runner' |
There was a problem hiding this comment.
Reporter: ESLint
Rule: eslint.rules.object-curly-spacing
Severity: ERROR
File: src/store/clipboard/index.ts L6
There should be no space after '{'. (object-curly-spacing)
| import { IRootState } from '@/store' | ||
| import { IPrompt } from '@floip/flow-runner'; | ||
| import { router } from '@/router' | ||
| import { IPrompt } from '@floip/flow-runner' |
There was a problem hiding this comment.
Reporter: ESLint
Rule: eslint.rules.object-curly-spacing
Severity: ERROR
File: src/store/clipboard/index.ts L6
There should be no space before '}'. (object-curly-spacing)
| } from 'vuex' | ||
| import { IRootState } from '@/store' | ||
| import { IPrompt } from '@floip/flow-runner'; | ||
| import { router } from '@/router' |
There was a problem hiding this comment.
Reporter: ESLint
Rule: eslint.rules.object-curly-spacing
Severity: ERROR
File: src/store/clipboard/index.ts L5
There should be no space after '{'. (object-curly-spacing)
| } from 'vuex' | ||
| import { IRootState } from '@/store' | ||
| import { IPrompt } from '@floip/flow-runner'; | ||
| import { router } from '@/router' |
There was a problem hiding this comment.
Reporter: ESLint
Rule: eslint.rules.object-curly-spacing
Severity: ERROR
File: src/store/clipboard/index.ts L5
There should be no space before '}'. (object-curly-spacing)
| @@ -2,7 +2,8 @@ import { | |||
| ActionTree, GetterTree, Module, MutationTree, | |||
| } from 'vuex' | |||
| import { IRootState } from '@/store' | |||
There was a problem hiding this comment.
Reporter: ESLint
Rule: eslint.rules.object-curly-spacing
Severity: ERROR
File: src/store/clipboard/index.ts L4
There should be no space after '{'. (object-curly-spacing)
| @@ -2,7 +2,8 @@ import { | |||
| ActionTree, GetterTree, Module, MutationTree, | |||
| } from 'vuex' | |||
| import { IRootState } from '@/store' | |||
There was a problem hiding this comment.
Reporter: ESLint
Rule: eslint.rules.object-curly-spacing
Severity: ERROR
File: src/store/clipboard/index.ts L4
There should be no space before '}'. (object-curly-spacing)
# Resolved Conflicts: # src/components/interaction-designer/toolbar/TreeBuilderToolbar.vue # src/views/InteractionDesigner.vue # tests/unit/__snapshots__/storybook.spec.ts.snap
| @@ -496,12 +495,7 @@ export default class TreeBuilderToolbar extends mixins(Routes, Permissions, Lang | |||
| return (this.can('view-result-totals') && this.isFeatureViewResultsEnabled) | |||
There was a problem hiding this comment.
Reporter: ESLint
Rule: eslint.rules.@typescript-eslint/strict-boolean-expressions
Severity: ERROR
File: src/components/interaction-designer/toolbar/TreeBuilderToolbar.vue L495
Unexpected any value in conditional. An explicit comparison or type cast is required. (@typescript-eslint/strict-boolean-expressions)
# Resolved Conflicts: # src/views/InteractionDesigner.vue
|
Found 24 violations: Reporter: ESLint Unexpected any value in conditional. An explicit comparison or type cast is required. (@typescript-eslint/strict-boolean-expressions)
Reporter: ESLint There should be no space after '{'. (object-curly-spacing)
Reporter: ESLint There should be no space before '}'. (object-curly-spacing)
Reporter: ESLint There should be no space after '{'. (object-curly-spacing)
Reporter: ESLint There should be no space before '}'. (object-curly-spacing)
Reporter: ESLint There should be no space after '{'. (object-curly-spacing)
Reporter: ESLint There should be no space before '}'. (object-curly-spacing)
Reporter: ESLint Unexpected any value in conditional. An explicit comparison or type cast is required. (@typescript-eslint/strict-boolean-expressions)
Reporter: ESLint There should be no space after '{'. (object-curly-spacing)
Reporter: ESLint There should be no space before '}'. (object-curly-spacing)
Reporter: ESLint This line has a length of 158. Maximum allowed is 140. (max-len)
Reporter: ESLint Unexpected any value in conditional. An explicit comparison or type cast is required. (@typescript-eslint/strict-boolean-expressions)
Reporter: ESLint Unexpected any value in conditional. An explicit comparison or type cast is required. (@typescript-eslint/strict-boolean-expressions)
Reporter: ESLint Expected a comma. (@typescript-eslint/member-delimiter-style)
Reporter: ESLint There should be no space after '{'. (object-curly-spacing)
Reporter: ESLint There should be no space before '}'. (object-curly-spacing)
Reporter: ESLint There should be no space after '{'. (object-curly-spacing)
Reporter: ESLint There should be no space before '}'. (object-curly-spacing)
Reporter: ESLint There should be no space after '{'. (object-curly-spacing)
Reporter: ESLint There should be no space before '}'. (object-curly-spacing)
Reporter: ESLint Unexpected any value in conditional. An explicit comparison or type cast is required. (@typescript-eslint/strict-boolean-expressions)
Reporter: ESLint Unexpected any value in conditional. An explicit comparison or type cast is required. (@typescript-eslint/strict-boolean-expressions)
Reporter: ESLint Missing return type on function. (@typescript-eslint/explicit-module-boundary-types)
Reporter: ESLint 'class' should be on a new line. (vue/max-attributes-per-line)
|
| if (this.$route.meta?.isBlockEditorShown) { | ||
| this.setIsBlockEditorOpen(true) | ||
| } | ||
| if (this.$route.name === 'flow-simulator' && this.hasSimulator()) { |
There was a problem hiding this comment.
Reporter: ESLint
Rule: eslint.rules.@typescript-eslint/strict-boolean-expressions
Severity: ERROR
File: src/views/InteractionDesigner.vue L209
Unexpected any value in conditional. An explicit comparison or type cast is required. (@typescript-eslint/strict-boolean-expressions)
| @@ -221,6 +206,9 @@ export default { | |||
| if (this.$route.meta?.isBlockEditorShown) { | |||
There was a problem hiding this comment.
Reporter: ESLint
Rule: eslint.rules.@typescript-eslint/strict-boolean-expressions
Severity: ERROR
File: src/views/InteractionDesigner.vue L206
Unexpected any value in conditional. An explicit comparison or type cast is required. (@typescript-eslint/strict-boolean-expressions)
| ...mapGetters('clipboard', ['isSimulatorActive']), | ||
| ...mapGetters('clipboard', ['hasSimulator', 'isSimulatorActive']), | ||
|
|
||
| jsKey() { |
There was a problem hiding this comment.
Reporter: ESLint
Rule: eslint.rules.@typescript-eslint/explicit-module-boundary-types
Severity: WARN
File: src/views/InteractionDesigner.vue L138
Missing return type on function. (@typescript-eslint/explicit-module-boundary-types)
|
|
||
| <div | ||
| class="tree-sidebar"> | ||
| <div v-if="$route.name === 'flow-simulator' && hasSimulator" class="tree-sidebar"> |
There was a problem hiding this comment.
Reporter: ESLint
Rule: eslint.rules.vue/max-attributes-per-line
Severity: WARN
File: src/views/InteractionDesigner.vue L14
'class' should be on a new line. (vue/max-attributes-per-line)
safydy-r
left a comment
There was a problem hiding this comment.
Sorry for the late review here @poojahpatil90 , I'm approving.
I'd like to catchup with you about my question on route params later.
Thanks
| }) | ||
| return { | ||
| params: { | ||
| id: this.activeFlow?.uuid, |
There was a problem hiding this comment.
@poojahpatil90
Sorry, I still don't understand, hihi 😸 . But it seems you know what you are doing here. I just love to have a quick call to understand why we still providing mode: this.isEditable ? 'view' : 'edit' here and not component: 'interaction-designer',.
@safydy-r Thanks for approving! I'll be happy to explain my thoughts behind these changes. Feel free to setup a call with me when it is convenient to you :) |
Uh oh!
There was an error while loading. Please reload this page.