-
Notifications
You must be signed in to change notification settings - Fork 0
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
Changes to allow for a single form per tile #160
Conversation
0d8dc76
to
0175346
Compare
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.
looks good! Just some minor stuff
@@ -26,32 +26,19 @@ defineProps<{ | |||
mode?: DataComponentMode; | |||
}>(); | |||
|
|||
defineEmits([OPEN_EDITOR]); | |||
const emit = defineEmits([OPEN_EDITOR, "updated"]); |
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.
nit: emits
to match the rest of the app
), | ||
}); | ||
} | ||
await updateSchemeNamespace( |
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.
removing error handling?
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.
unintentional. looks like some kind of merge issue. Thanks for the catch.
schemeComponents.find((component) => { | ||
return component.id === newValue.activeTab; | ||
}) || schemeComponents[0]; | ||
console.log(currentEditor.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.
nit: remove console.log
currentEditor.value = | ||
schemeComponents.find((component) => { | ||
return component.id === newValue.activeTab; | ||
}) || schemeComponents[0]; |
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.
could this fallback end up being confusing for the user?
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.
yeah, actually was made for tabs. Now that tabs are gone, it should be also.
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.
looks good just a few minor things fell out
arches_lingo/src/arches_lingo/components/scheme/report/SchemeNamespace.vue
Outdated
Show resolved
Hide resolved
arches_lingo/src/arches_lingo/components/scheme/report/SchemeNamespace.vue
Show resolved
Hide resolved
arches_lingo/src/arches_lingo/components/scheme/editor/SchemeEditor.vue
Outdated
Show resolved
Hide resolved
@@ -86,7 +86,7 @@ const getRef = (el: object | null, index: number) => { | |||
<SchemeEditor | |||
v-if="editorTab" | |||
:editor-max="sectionVisible" | |||
:active-tab="editorTab" | |||
:editor-form="editorTab" |
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.
still using tab
language here?
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.
purged. 🏓
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.
🏓
@@ -34,7 +34,7 @@ const onClose = () => { | |||
}; | |||
|
|||
const onOpenEditor = (tab: string, tileId: string) => { | |||
editorTab.value = tab; | |||
editorForm.value = tab; |
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.
tab
language here too. Sorry to be nitpicky but this is the ticket that purges the concept of tabs from the frontend yeah?
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.
Yeap. Thanks for finding those. 🏓
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.
🚀
let the collapsing begin. |
* Changes to allow for a single form per tile * move section types to prep for conflict * PR feedback * pr feedback * purge tab * purge more tabs
* Changes to allow for a single form per tile * move section types to prep for conflict * PR feedback * pr feedback * purge tab * purge more tabs
Allows for a single editor per tile, removes tabbed interface.