-
Notifications
You must be signed in to change notification settings - Fork 19
VMO-3852 Support deep linking for simulator + VMO-3687 Simulator on read only view #75
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
base: master
Are you sure you want to change the base?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't this be treeId: this.activeFlow?.uuid ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And I'm a bit confused why we removed component: ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
| { | ||
| path: 'simulator', | ||
| name: 'flow-simulator', | ||
| meta: { isSidebarShown: true }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reporter: ESLint
Rule: eslint.rules.object-curly-spacing
Severity: ERROR
File: src/router/trees.js L38
There should be no space after '{'. (object-curly-spacing)
| { | ||
| path: 'simulator', | ||
| name: 'flow-simulator', | ||
| meta: { isSidebarShown: true }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reporter: ESLint
Rule: eslint.rules.object-curly-spacing
Severity: ERROR
File: src/router/trees.js L38
There should be no space before '}'. (object-curly-spacing)
| addToBlocksData({ state }, data: BlocksData) { | ||
| state.blocksData.push(data) | ||
| }, | ||
| removeFromBlocksData({ state }, index: number) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
| addToBlocksData({ state }, data: BlocksData) { | ||
| state.blocksData.push(data) | ||
| }, | ||
| removeFromBlocksData({ state }, index: number) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
| name: routeName, | ||
| }) | ||
| }, | ||
| resetBlocksData({ state }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reporter: ESLint
Rule: eslint.rules.object-curly-spacing
Severity: ERROR
File: src/store/clipboard/index.ts L48
There should be no space after '{'. (object-curly-spacing)
| name: routeName, | ||
| }) | ||
| }, | ||
| resetBlocksData({ state }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reporter: ESLint
Rule: eslint.rules.object-curly-spacing
Severity: ERROR
File: src/store/clipboard/index.ts L48
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
| } | ||
|
|
||
| export const actions: ActionTree<IClipboardState, IRootState> = { | ||
| setSimulatorActive({ commit }, value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
| } | ||
|
|
||
| export const actions: ActionTree<IClipboardState, IRootState> = { | ||
| setSimulatorActive({ commit }, value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
| import { | ||
| ActionTree, GetterTree, Module, MutationTree, | ||
| } from 'vuex' | ||
| import { IRootState } from '@/store' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
| import { | ||
| ActionTree, GetterTree, Module, MutationTree, | ||
| } from 'vuex' | ||
| import { IRootState } from '@/store' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
| } | ||
| get canViewResultsTotals(): any { | ||
| return (this.can('view-result-totals') && this.isFeatureViewResultsEnabled) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
| if (field) { | ||
| scrollBehavior(this.$route) | ||
| } | ||
| if (this.$route.meta?.isBlockEditorShown) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.