-
Notifications
You must be signed in to change notification settings - Fork 144
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
Vue etl manager #10822
base: dev/7.6.x
Are you sure you want to change the base?
Vue etl manager #10822
Conversation
following |
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.
This is a good start! 👍
There's a few changes I think should be made that will very much aide in component decomposition and the single-responsibility principle. I've only broken down the first screen, I'll circle back to the others depending on how this goes.
There seems to be a lot of logic crammed into vue-data-manager.vue
. Below I've renamed it to DataManager
. IMO it should be pretty simple, with most of the logic moved closer to the components that use them. This is how I see the breakdown of the first screen:
DataManager.vue
is just responsible for loading the tab content and keeping track of the current tab. A rough estimate would be
<script setup>
import { ref } from 'vue';
import Tabs from '@/componets/DataManager/Tabs.vue';
import StartTabContent from '@/components/DataManager/StartTabContent.vue';
etc
etc
const tabs = [
{ id: 1, title: 'Start', content: 'StartTabContent' },
etc
etc
];
const currentContent = ref('StartTabContent');
const handleTabChange = (tabId) => {
const selectedTab = tabs.find(tab => tab.id === tabId);
currentContent.value = selectedTab.content;
};
</script>
<template>
<div>
<Tabs :tabs="tabs" @change="handleTabChange" />
<component :is="currentContent" />
</div>
</template>
Tab
is just responsible for emitting an event when clicked ( && the click is valid ) , and showing state (eg selected/disabled, etc )
<script setup>
import { defineEmits } from 'vue';
const props = defineProps({
title: String,
isActive: Boolean,
tabId: Number
});
const emit = defineEmits(['tab-selected']);
const selectTab = () => {
if (something valid) {
emit('tab-selected', props.tabId);
}
};
</script>
<template>
<button @click="selectTab" :class="{ active: isActive }">
Title goes here
</button>
</template>
Tabs
just shows the list of tabs, and manages which tab is active
<script setup>
import { ref } from 'vue';
import Tab from '@/components/DataManager/Tab.vue';
const props = defineProps({
tabs: Array
});
const currentTab = ref(props.tabs[0].id);
const selectTab = (tabId) => {
currentTab.value = tabId;
emit('change', tabId);
};
const emit = defineEmits(['change']);
</script>
<template>
<div>
<Tab
v-for="tab in tabs"
:key="tab.id"
:title="tab.title"
:isActive="currentTab === tab.id"
:tabId="tab.id"
@tab-selected="selectTab"
/>
</div>
</template>
StartTabContent
just houses FilterModules
and ModuleCards
, and acts as a go-between for filter data between the two.
FilterModules
is basically the ModuleSearch
component you currently have. This is an aside but since we're rewriting the thing the Import, Edit, Export
buttons aren't really buttons, right? Just filters. IMO they shouldn't be buttons but either disappear entirely or be radios or options to the filter box. They could be included in the component that way. But this is not the main point of the review 😅
ModuleCards
houses ModuleCard
, much like Tabs
and Tab
. There is no state to manage here and this could just be a <template>
component.
ModuleCard
is essentially display-only , and it emits an event on click that DataManager
listens for so it can change the active TabContent
component.
This is a general rough-out and meant to start a discussion about design patterns. Please let me know how you feel and if you have any questions about the logic to it! 😄
@ryan86 in the plugins table in the db, what's the value for |
@whatisgalen - it looks like your guess node mappings functionality would be easy to included in this before roll out. value is |
First cut at converting etl manager to vue.