Skip to content

💾 Feat/25 Add Iobio components#207

Merged
demariadaniel merged 32 commits into
iobiofrom
feat/25-add-iobio-components
Aug 30, 2024
Merged

💾 Feat/25 Add Iobio components#207
demariadaniel merged 32 commits into
iobiofrom
feat/25-add-iobio-components

Conversation

@demariadaniel

@demariadaniel demariadaniel commented Jul 30, 2024

Copy link
Copy Markdown
  • All Iobio components rendering
  • Async Iobio Components imports for SSR page refresh (now handled in package)
  • next.config DOM shims for handling web component features
  • Extremely basic file/visuals table toggle for testing

https://app.zenhub.com/workspaces/overture-stack-5d2e058ff67cc800011fee6b/issues/gh/overture-stack/iobio-components/25

@demariadaniel demariadaniel changed the title WIP: Feat/25 add iobio components 💾 Feat/25 Add Iobio components Aug 28, 2024
Comment thread components/pages/explorer/BamTable.tsx Outdated
Comment thread next.config.js
const dom = new JSDOM('', { url: 'http://localhost/' });

// @ts-ignore
global.window = dom.window;

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Obviously ts-ignore is not ideal, I'll take a second look at this

@justincorrigible justincorrigible Aug 30, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

was this JSDOM necessary for the SSR issues? if so, perhaps it can be removed now

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

No I've tested this several times and it causes errors when removed. Might be worth a deep dive, there may be a better Next-centric approach.

@demariadaniel demariadaniel Aug 30, 2024

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

To be clear, yes JSDOM is solving part of the web components / SSR issue, plus the client side async import.

@demariadaniel demariadaniel changed the base branch from develop to iobio August 30, 2024 14:05
Comment thread components/pages/explorer/BamTable.tsx Outdated
Comment thread components/pages/explorer/BamTable.tsx Outdated
<h2>Bam.iobio</h2>
<>
<IobioDataBroker
alignmentUrl={'https://s3.amazonaws.com/iobio/NA12878/NA12878.autsome.bam'}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This value should be abstracted as an env var.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes - this is a specific file URL, so it will either live in a TableContext or in the URL query eventually
It's hard coded because implementation is still TBD

Comment thread next.config.js
global.document = global.window.document;
global.Element = global.window.Element;
global.localStorage = global.window.localStorage;
global.navigator = global.window.navigator;

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

These global.* values are all undefined when the iobio script is loaded on the server

>
<TableContextProvider>
<h2>Bam.iobio</h2>
<>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do we need this empty tag?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Not necessarily, removed

Comment thread components/pages/explorer/BamTable.tsx Outdated

return useMemo(
() => (
<>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Don't need this empty tag, there is an <article> tag around all the content here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Removed

@demariadaniel demariadaniel merged commit ae8f947 into iobio Aug 30, 2024
@demariadaniel demariadaniel deleted the feat/25-add-iobio-components branch August 30, 2024 18:10
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.

3 participants