Julenmendieta/milab 6240 minor fixes - #7
Conversation
- Use right plot title format and naming
There was a problem hiding this comment.
Code Review
This pull request updates dependency versions, renames UI sections to 'Property Relationships' and 'Property Distribution', and modifies the layout of the histogram and scatter pages. Review feedback highlights that using export type * is discouraged due to potential name collisions and that removing the refsWithEnrichments option might prevent the correct identification of columns from enriched datasets. Furthermore, the removal of PlBlockPage wrappers in the UI components is flagged for breaking layout consistency and standard behavior within the platform.
| import type { BlockArgs, WorkflowInfo } from "./types"; | ||
|
|
||
| export type { BlockArgs, BlockData, WorkflowInfo, WorkflowMode, WorkflowReceptor } from "./types"; | ||
| export type * from "@milaboratories/helpers"; |
| .output("inputOptions", (ctx) => | ||
| ctx.resultPool.getOptions(inputAnchorSpecs, { refsWithEnrichments: true }), | ||
| ) | ||
| .output("inputOptions", (ctx) => ctx.resultPool.getOptions(inputAnchorSpecs)) |
There was a problem hiding this comment.
Removing { refsWithEnrichments: true } may prevent the block from correctly identifying and using columns that originate from enriched datasets. This option is typically required in Platforma to ensure that the UI can resolve all relevant data columns in the result pool.
| .output("inputOptions", (ctx) => ctx.resultPool.getOptions(inputAnchorSpecs)) | |
| .output("inputOptions", (ctx) => ctx.resultPool.getOptions(inputAnchorSpecs, { refsWithEnrichments: true })) |
| <GraphMaker | ||
| v-model="app.model.data.graphStateHistogram" | ||
| chart-type="histogram" | ||
| :p-frame="app.model.outputs.propertiesPfHandle" | ||
| :default-options="defaultOptions" | ||
| :data-column-predicate="dataColumnPredicate" | ||
| :status-text="{ | ||
| noPframe: { title: 'Select an input dataset on the Main tab to plot.' }, | ||
| }" | ||
| /> |
There was a problem hiding this comment.
Removing the PlBlockPage wrapper and its #title template breaks UI consistency and layout within the Platforma ecosystem. PlBlockPage provides standard padding, scrolling behavior, and header integration. It is recommended to keep it and update the title to match the new section label ("Property Distribution"). Note: You will also need to restore the PlBlockPage import.
<PlBlockPage>
<template #title>Property Distribution</template>
<GraphMaker
v-model="app.model.data.graphStateHistogram"
chart-type="histogram"
:p-frame="app.model.outputs.propertiesPfHandle"
:default-options="defaultOptions"
:data-column-predicate="dataColumnPredicate"
:status-text="{
noPframe: { title: 'Select an input dataset on the Main tab to plot.' },
}"
/>
</PlBlockPage>
| <GraphMaker | ||
| v-model="app.model.data.graphStateScatter" | ||
| chart-type="scatterplot-umap" | ||
| :p-frame="app.model.outputs.propertiesPfHandle" | ||
| :default-options="defaultOptions" | ||
| :default-palette="{ categorical: 'bright' }" | ||
| :data-column-predicate="dataColumnPredicate" | ||
| :status-text="{ | ||
| noPframe: { title: 'Select an input dataset on the Main tab to plot.' }, | ||
| }" | ||
| /> |
There was a problem hiding this comment.
Removing the PlBlockPage wrapper breaks UI consistency and layout. It is recommended to retain it and update the title to "Property Relationships" to align with the updated navigation labels. Note: You will also need to restore the PlBlockPage import.
<PlBlockPage>
<template #title>Property Relationships</template>
<GraphMaker
v-model="app.model.data.graphStateScatter"
chart-type="scatterplot-umap"
:p-frame="app.model.outputs.propertiesPfHandle"
:default-options="defaultOptions"
:default-palette="{ categorical: 'bright' }"
:data-column-predicate="dataColumnPredicate"
:status-text="{
noPframe: { title: 'Select an input dataset on the Main tab to plot.' },
}"
/>
</PlBlockPage>
No description provided.