#79 / Library Updates for UI by demariadaniel · Pull Request #80 · overture-stack/iobio-components

demariadaniel · 2025-08-06T21:22:04Z

#79 Feat: Changes to Support UI Changes in: https://github.com/overture-stack/stage/pull/253/files

Summary

#79 contains changes to fix the rendering of Iobio Labels
This requires an update to iobio-charts @0.26.0 and new JSX definitions
This PR bundles the library update with several other package improvements

Issues

#(79)](Bug Fixes for Visualization Selector Modal #79)

Description of Changes

Updates iobio-charts to @ 0.26
Adds component definitions for Iobio LabelInfoButton and Panel
Updates use of label prop
Adds default copy for LabelInfoButton from Iobio site
Adds Zod Validation and improves Type definitions
Moves Score related files out of Metadata generator folder
Changes Score configuration

Readiness Checklist

Self Review
- I have performed a self review of code
- I have run the application locally and manually tested the feature
- I have checked all updates to correct typos and misspellings
Formatting
- Code follows the project style guide
- Autmated code formatters (ie. Prettier) have been run
Local Testing
- Successfully built all packages locally
- Successfully ran all test suites, all unit and integration tests pass
Updated Tests
- Unit and integration tests have been added that describe the bug that was fixed or the features that were added
Documentation
- All new environment variables added to .env.schema file and documented in the README
- All changes to server HTTP endpoints have open-api documentation
- All new functions exported from their module have TSDoc comment documentation

…mments

…ome import.meta logic

…ents into 79/bug-fixes-for-modal

demariadaniel · 2025-08-07T14:01:16Z

Moved out of utils/elasticMetadataClient

joneubank · 2025-08-07T16:09:49Z

+	if (!FileMetaDataSchema.safeParse(indexFileMetadata))
+		console.error(`Error retrieving Index file from Score with object_id: ${fileObjectId}, results may be inaccurate`);
+
+	return { scoreFileMetadata, indexFileMetadata };


Most importantly, FileMetaDataSchema.safeParse(...) ALWAYS is truthy, it returns an object. you want to check FileMetaDataSchema.safeParse(...).success.

Following that, we should be returning the data from the parse result, not the original (untyped) response data:

const metadataParseResult = FileMetaDataSchema.safeParse(indexFileMetadata); if(!metadataParseResult.success) { console.error(`Error retrieving Index file from Score with object_id: ${fileObjectId}, results may be inaccurate`); // return something that indicates our data wasn't available, otherwise the consumer of this function will process data that is the wrong shape and will break. } return { scoreFileMetadata, indexFileMetadata: metadataParseResult.data };

Sorry let me revise this, I followed up on some of your previous PR comments then the changes got lost in the shuffle and may have rushed a few revisions

joneubank · 2025-08-07T16:34:31Z

+export const FileMetaDataSchema = zod.object({
+	objectId: zod.string(),
+	objectKey: zod.string().optional(),
+	objectMd5: zod.string().optional(),
+	objectSize: zod.number().optional(),
+	parts: zod.array(
+		zod.object({
+			md5: zod.string().nullable().optional(),
+			offset: zod.number().optional(),
+			partNumber: zod.number().optional(),
+			partSize: zod.number().optional(),
+			url: zod.string(),
+		}),
+	),
+	uploadId: zod.string().optional(),
+});


This schema is disconnected from the type we declared in scoreFileTypes. If one changes and the other doesn't, then we are validating the incorrect type.

The way to fix this is to move this schema into the scoreFileTypes.ts file, and then to use zod.infer to generate the type:

export const fileMetaDataSchema = zod.object({ objectId: zod.string(), objectKey: zod.string().optional(), objectMd5: zod.string().optional(), objectSize: zod.number().optional(), parts: zod.array( zod.object({ md5: zod.string().nullable().optional(), offset: zod.number().optional(), partNumber: zod.number().optional(), partSize: zod.number().optional(), url: zod.string(), }), ), uploadId: zod.string().optional(), }); export type FileMetaData = zod.infer<typeof fileMetaDataSchema>;

ciaranschutte

looking really good overall - couple of q's , couple of style fix ups

ciaranschutte · 2025-08-07T14:03:01Z

 }): Promise<{ fileUrl: string; fileName?: string; indexFileUrl?: string }> => {
 	const { documentId } = esConfig;
-	const elasticDocument = searchResult._source;
 	if (elasticDocument.file_type && !BamFileExtensions.includes(elasticDocument.file_type)) {


simplify to !BamFileExtensions.includes(elasticDocument.file_type) ?

This is for TS b/c BamFileExtensions does not include undefined

ciaranschutte · 2025-08-07T14:03:17Z

 const authKey = process.env.ES_AUTH_KEY;
 const esHost = process.env.ES_HOST_URL;
-if (!(authKey && esHost)) throw new Error('Required .env configuration values are missing');
+if (!(authKey && esHost)) throw new Error('Required ElasticSearch .env configuration values are missing');


wrap the "if"
good giving a better message - maybe we should go all the way and say no ES_AUTH_KEY or ES_HOST_URL?

Right -- added several wrapping {} brackets and improved error messages

ciaranschutte · 2025-08-07T14:03:41Z

+export const infoLabelPercentCopy: { [P in BamPercentKey]: string } = {
+	mapped_reads:


this is the current usage in a consumer, example from Stage:

<IobioPanel> <IobioLabelInfoButton label={displayNames[key]}> <div slot="content"> <p>{infoLabelPercentCopy[key]}</p> </div> </IobioLabelInfoButton> <IobioPercentBox percentKey={key} totalKey="total_reads" /> </IobioPanel>

None of these are really "default descriptions" because the consumer eg. Stage is selecting copy based on the key. There's some "library thinking" to be applied here and I'm not 100% clear what the end goal is.
if these are truly library level defaults, should they display when not given input from a consumer?

OK so some background info to provide here. iobio-charts exports the Label and InfoButton tooltip together as one component. The content is for the pop up modal. We don't have our own copy for the modal (I'm using what is on Iobio's site).
But the consumer might. So making these a default (display when there's no content) on our package side is a smart suggestion.

Restructured the info buttons to show standard display names and modal copy
This requires a bamKey string prop and overrides additional labels/children if provided

df454e6

ciaranschutte · 2025-08-07T14:03:57Z

+export interface IobioDataBrokerProps extends IobioElementProps {
+	'alignment-url': string;
+	'index-url'?: string;
+	server?: string;
+}


I think the props are camelCased ?
<IobioDataBroker alignmentUrl={fileUrl} indexUrl={indexFileUrl}...>

react component props should be camelCased while html elements (and web components) are kebab-cased

Shoot yeah there's some overlap here that's not correct, looking into it

This file is for JSX to recognize the web components. I've updated the names and definitions to be a bit more reflective (hasn't been looked at it in depth for about a year). The kebob case is intentional because those are the properties of the web components.

ciaranschutte · 2025-08-07T14:04:21Z

+
+export default IobioPanel;
+
+export type IobioPanelType = typeof IobioPanel;


What's the use case of this?
Is there a case where we want to provide a specific IobioPanel react node as a prop?

I think these were intended to have a Type representation for the components. But they are unused here and in Stage so they could probably be removed.

Naming conventions generally don't want a type to be named with Type, its redundant. There is no harm in having the type name be the same as the variable name (ie. IobioPanel).

@joneubank I removed all of these IobioComponentType definitions, they don't have an immediate use case:
e689473

joneubank

Looks pretty good, left two very small suggestions.

joneubank · 2025-08-11T20:39:46Z

+
+export type IobioInfoModalKeys = BamHistogramKey | BamPercentKey;
+
+export const infoModalCopy: { [K in IobioInfoModalKeys]: string } = {


Suggested change

export const infoModalCopy: { [K in IobioInfoModalKeys]: string } = {

export const infoModalCopy: Record<IobioInfoModalKeys, string> = {

joneubank · 2025-08-11T20:42:19Z

+	const urlParams = new URLSearchParams(scoreDownloadParams).toString();
+	try {
+		const scoreUrl = urlJoin(scoreApiUrl, scoreApiDownloadPath, objectId, `?${urlParams}`);
+		const response = (await fetch(scoreUrl)).json();


Combining .json() onto this line doesn't actually address the issue @ciaranschutte was pointing out, this now returns the Promise from the .json deserialization to the caller and requires them to handle those errors. If you want any errors from the .json call to be handled by the catch block in this function then you need to await the .json() call as well:

const response = await fetch(scoreUrl); const jsonData = await response.json(); return jsonData;

joneubank · 2025-08-11T20:43:01Z

+	uploadId: zod.string().optional(),
+});
+
+export type FileMetaData = zod.infer<typeof fileMetaDataSchema>;


Jon approved :thumbs-up:

demariadaniel added 30 commits July 17, 2025 10:04

Rename utils file

eb5580a

Utility Round 1

4c1c4fb

Use prompt for stats, additional formatting updates

49d88fb

Major reorg w/ Score additions

54bbbc0

CLI Setup w/ basic update

c439d31

Working Indexer CLI

759429b

Reorg & Update comments

a5a63d8

Fix stats script, update use of iobio-metadata file name, improved co…

e8d9196

…mments

Rename files

93df20e

Update Readme

90a7fd8

Correct Score Download

de856b5

Alphabetize env.schema

8a7a949

Improved Types

b8462ed

Base setup

16f1eff

Add bedFile arg to CLI

a5795e1

Tested with local urls

fe75ca2

Export function for UI use

7ddc6fa

Move around definitions & comments

114d957

ENV Changes for Stage

1e6ade3

Export Types

4cc5a2d

Use File URLs, Add Bed Files, Move Bed logic to statistics, reverts s…

9c163ee

…ome import.meta logic

Revert Vite env vars, needs review

f6bfaf7

Move score files out of metadata folder & some definitions to Stage

efc73bc

Separate Node/Browser Bed Definitions

0014800

First Pass JSX Components

de4a9fb

Remove percentBox label prop

3c20129

Merge branch 'main' of https://github.com/overture-stack/iobio-compon…

e9aaf04

…ents into 79/bug-fixes-for-modal

isBamKey changes

a80534b

Update iobio-charts

aff8329

Score Config Changes

19378d2

demariadaniel mentioned this pull request Aug 6, 2025

#79 / Accessibility & Style fixes for Modal overture-stack/stage#253

Merged

3 of 5 tasks

demariadaniel requested review from ciaranschutte, joneubank and justincorrigible August 6, 2025 22:12

demariadaniel commented Aug 7, 2025

View reviewed changes

joneubank reviewed Aug 7, 2025

View reviewed changes

ciaranschutte previously requested changes Aug 7, 2025

View reviewed changes

demariadaniel added 12 commits August 8, 2025 10:43

Fix copyright

b3c385b

Add brackets + improved errors

0f359ca

Score Tools response & comment updates

ffe1fdc

JS Constant casing

2cd0e81

camelCase objectId

22b2422

Update types & use const

5f2297f

Remove component Types

e689473

Move content slot into labelInfoButton

49a22c2

Updated fileMetaDataSchema

67ff86d

Updated Default Info Label

df454e6

Build error - update index exports

57a63cb

Add percentBox styles dependency

11117e6

demariadaniel requested a review from joneubank August 11, 2025 16:14

joneubank previously approved these changes Aug 11, 2025

View reviewed changes

Use Record for InfoModal keys

6a3065b

demariadaniel dismissed joneubank’s stale review via 6a3065b August 11, 2025 21:22

Correct await response.json usage

50ff2df

joneubank approved these changes Aug 11, 2025

View reviewed changes

demariadaniel merged commit 90fc1f3 into main Aug 12, 2025

demariadaniel deleted the 79/bug-fixes-for-modal branch August 12, 2025 12:46

demariadaniel mentioned this pull request Aug 13, 2025

RC 0.0.5 #82

Closed

5 tasks

		export const infoLabelPercentCopy: { [P in BamPercentKey]: string } = {
		mapped_reads:


		export default IobioPanel;

		export type IobioPanelType = typeof IobioPanel;


		export type IobioInfoModalKeys = BamHistogramKey \| BamPercentKey;

		export const infoModalCopy: { [K in IobioInfoModalKeys]: string } = {

	export const infoModalCopy: { [K in IobioInfoModalKeys]: string } = {
	export const infoModalCopy: Record<IobioInfoModalKeys, string> = {

Conversation

demariadaniel commented Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Issues

Description of Changes

Readiness Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joneubank Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ciaranschutte left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

joneubank left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

demariadaniel commented Aug 6, 2025 •

edited

Loading

joneubank Aug 7, 2025 •

edited

Loading