323 vue3 topology display #359
Conversation
Align TimeSeriesDisplay with TopologyDisplay
Rename Displays to Views to make distinction between components and views clearer
Move functionality from composition utility to lib
Use SpatialDisplay in SpatialDisplayView
src/lib/topology/index.ts
Outdated
| * @param {TopologyNode[] | undefined} nodes - An array of TopologyNode objects or undefined. | ||
| * @param {Map<string, TopologyNode>} topologyMap - A Map used to store the topology nodes. | ||
| */ | ||
| export function createTopologyMap( |
There was a problem hiding this comment.
Maybe make this a function that returns the Map? The recursion can then be achieved with an internal function, so the caller does not need to know about it.
| */ | ||
| export function useTopologyNodes( | ||
| export function useDisplayConfig( |
There was a problem hiding this comment.
This directory should probably be renamed to useDisplayConfig
src/services/useWms/index.ts
Outdated
| } | ||
|
|
||
| function loadTimes(): void { | ||
| let valueDates: Date[] |
There was a problem hiding this comment.
It looks like there's a few cases where [] is assigned to valueDates that can be removed if you just initialise valueDates to an empty array here?
src/services/useWms/index.ts
Outdated
| capabilities.layers[0] | ||
| } | ||
| } catch (error) { | ||
| console.log(error) |
There was a problem hiding this comment.
Do we need better error handling than this? I would suggest using at least console.error instead of console.log
| ) | ||
|
|
||
| const plotIds = computed(() => { | ||
| if (displays.value?.length) { |
There was a problem hiding this comment.
I'm not a big fan of implicit boolean conversion of numbers... maybe displays.value && displays.value.length > 0?
There was a problem hiding this comment.
Feel free to ignore this (but fix it if you like ;)), just noticed it's code that was moved around...
| const plotIds = computed(() => { | ||
| if (displays.value?.length) { | ||
| const ids = displays.value.map((d) => { | ||
| return d.title |
There was a problem hiding this comment.
Braces and return are not necessary, also you can return the result of the map directly instead of using an intermediate variable.
There was a problem hiding this comment.
Feel free to ignore this (but fix it if you like ;)), just noticed it's code that was moved around...
src/views/TopologyDisplayView.vue
Outdated
| createTopologyMap(nodes.value, topologyMap.value) | ||
| }) | ||
|
|
||
| function anyChildNodeIsVisible(nodes: TopologyNode[] | undefined): boolean { |
There was a problem hiding this comment.
Should checking for undefined not be the caller's responsibility?
src/views/TopologyDisplayView.vue
Outdated
| const topologyMap = ref(new Map<string, TopologyNode>()) | ||
| getTopologyNodes().then((response) => { | ||
| nodes.value = response | ||
| createTopologyMap(nodes.value, topologyMap.value) |
There was a problem hiding this comment.
Do we never need to update the topology?
src/views/TopologyDisplayView.vue
Outdated
|
|
||
| function anyChildNodeIsVisible(nodes: TopologyNode[] | undefined): boolean { | ||
| if (nodes === undefined) return false | ||
| for (const node of nodes) { |
There was a problem hiding this comment.
This function can be simplified as return nodes.some(topologyNodeIsVisible)
|
|
||
| watch(nodes, updateItems) | ||
|
|
||
| watchEffect(() => { |
There was a problem hiding this comment.
It would be nice if this had a comment with a brief description on what it is watching for and what it will update
7fdd5fe to
593fc4f
Compare
No description provided.