Skip to content

Julenmendieta/milab 6240 minor fixes - #7

Merged
julenmendieta merged 6 commits into
mainfrom
julenmendieta/MILAB-6240_minorFixes
May 8, 2026
Merged

julenmendieta merged 6 commits into
mainfrom
julenmendieta/MILAB-6240_minorFixes

Conversation

@julenmendieta

Copy link
Copy Markdown
Contributor

No description provided.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread model/src/index.ts
import type { BlockArgs, WorkflowInfo } from "./types";

export type { BlockArgs, BlockData, WorkflowInfo, WorkflowMode, WorkflowReceptor } from "./types";
export type * from "@milaboratories/helpers";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using export type * is generally discouraged as it can lead to name collisions and unnecessarily exposes the entire internal type surface of the helper library. It is better to explicitly export only the specific types required by the UI or other consumers.

Comment thread model/src/index.ts
.output("inputOptions", (ctx) =>
ctx.resultPool.getOptions(inputAnchorSpecs, { refsWithEnrichments: true }),
)
.output("inputOptions", (ctx) => ctx.resultPool.getOptions(inputAnchorSpecs))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
.output("inputOptions", (ctx) => ctx.resultPool.getOptions(inputAnchorSpecs))
.output("inputOptions", (ctx) => ctx.resultPool.getOptions(inputAnchorSpecs, { refsWithEnrichments: true }))

Comment on lines +31 to +40
<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.' },
}"
/>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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>

Comment on lines +40 to +50
<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.' },
}"
/>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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>

@julenmendieta
julenmendieta added this pull request to the merge queue May 8, 2026
Merged via the queue into main with commit 3a3408e May 8, 2026
9 checks passed
@julenmendieta
julenmendieta deleted the julenmendieta/MILAB-6240_minorFixes branch May 8, 2026 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant