-
Notifications
You must be signed in to change notification settings - Fork 629
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
Migrate from san to vue #281
Conversation
Add App.vue Add AppMenu.vue
* Add vuetify framework Update AppMenu.vue to include the basic menu item. * use name as key.
* Add vuetify framework Update AppMenu.vue to include the basic menu item. * use name as key. * Add Scalars, Images, Histograms, Graph. * Switch to use the stylus language for CSS * Port functions from Scalars.san to Scalars.vue
* Add vuetify framework Update AppMenu.vue to include the basic menu item. * use name as key. * Add Config Vue
* Fix warnings from Vue compiler * use default prop instead of duplicate data and prop * use function instead of shorthand declaration with data * refract naming style, fix scalar options * Fix config UI * Add chart and chart page and integrate to scalar * update with varun and jeff's comments
* Add vuetify framework Update AppMenu.vue to include the basic menu item. * use name as key. * Add the Config.vue for Images * Add the CharPage.vue and Image.vue * Hook up the config to the image.vue so that the image height and width can update
* Watch tagInfo changes to update chart and fix tooltip issue * Add expand panel and maintain isShow state in the panel * Add Expand Panel in Image
* Add vuetify framework Update AppMenu.vue to include the basic menu item. * use name as key. * Add Watch on the groupNameReg
* Make sure to clear the chart before each use. * Add key for v-for loop.
-Apply theme color in Vuetify -Add main.styl to override Vuetify default stylus variables -Add relative path in web pack config to find variables.styl
* Fix multiple scalar issues: -download link / outliers should take boolean value instead of array -download link now show proper selector of runsItem and download button -Use tag as key to prevent chart re-rendering -Add missing label in scalar config and fix UI * simply use index for key because scalar has different tag info structure from image and histogram
}, | ||
methods: { | ||
handleHeadClick() { | ||
this.toogleShow(); |
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.
toggle
<div class="visual-dl-expand-panel"> | ||
<h3 | ||
class="visual-dl-expand-head" | ||
@click="handleHeadClick()" |
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.
Since this click only toggles show/hide, we can write it simply as below:
@click="isShow = !isShow"
@@ -1,7 +1,9 @@ | |||
import Notification from './Notification'; | |||
import NotificationItem from './NotificationItem'; | |||
//TODO: Migrate this part to Vue |
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.
Can this file be moved?
frontend/src/graph/ui/Chart.vue
Outdated
|
||
getOriginChartsData() { | ||
getPluginGraphsGraph().then(({status, data}) => { | ||
if (+status === 0 && data.url) { |
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.
Is the '+' an error?
Also we should add handler for error promise later
frontend/src/graph/ui/Config.vue
Outdated
|
||
export default { | ||
props:['config'], | ||
data() { |
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.
data() is not used thus should be removed.
@daming-lu good catch regarding the download button bug. This is because "train" run is empty and nothing can be downloaded, same issue before the migration. I think the fix should let the selector only displays non empty chart. |
-Dropdown should only display non empty runs -Select first run by default -Remove drop down if all are empty runs -Refract download type to runItemForDownload -Add train run and multiple tags in mock data
-Error comes in when trying to access elements inside empty data -When one of the runs has empty data, the chart will display nothing even other run has non empty data -Fix by checking length of data before accessing
}, | ||
methods: { | ||
handleItemClick: function (item) { | ||
this.selected = item.name |
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.
这里为何没有缩进,建议合入后改一下,小问题
Download image | ||
</v-btn> | ||
|
||
<v-slider label="Scale" |
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.
这个 label 属性可以写在下一行,格式统一
<script> | ||
import ExpandPanel from '../../common/component/ExpandPanel'; | ||
import Image from './Image'; | ||
//import Pagination from 'san-mui/Pagination'; |
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.
san-mui 相关的后面统一删了吧
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.
LGTM
We created Vue version of the VisualDL. Please take some time to take a look. Once it is merged, let's discuss how we can build interactive graph together.