#40, #41, #43 - add jbrowse circular #184
Conversation
Co-authored-by: Anders Richardsson <[email protected]>
…/dms-ui into 39-jbrowse-circular
…i into 39-jbrowse-circular
samrichca
changed the title
|
@justincorrigible noted about the strings. jenkins is unhappy because our jbrowse plugin isn't updated on npm, i'll follow up on slack |
| export const RepositoryTabsContext = createContext<RepositoryTabsContextInterface>({ | ||
| activeTab: undefined, | ||
| openTabs: [], | ||
| } as unknown as RepositoryTabsContextInterface); |
There was a problem hiding this comment.
has a TS error without 'as unknown as x'. we had this on another context provider before
There was a problem hiding this comment.
I'll let @joneubank chime in on that one first, but I'm confident there's a more maintainable way to handle this. Also curious to see where else this pattern was deemed necessary, so we can improve things there too.
There was a problem hiding this comment.
I mixed this up, I used unknown with the tables. there must be a fix here since it worked before without it, but I'm stumped at the moment
There was a problem hiding this comment.
I know youre fixing this but I just want to respond that there is no reason to have double type assertion. In this case we shouldn't be faking our type, instead just provide default values that match the expected type. We overwrite these in the tab implementation immediately anyways:
export const RepositoryTabsContext = createContext<RepositoryTabsContextInterface>({
handleSwitchTab: () => {},
handleCloseTab: () => {},
handleOpenTab: () => {},
handleUpdateTab: () => {},
openTabs: [],
});| position="left" | ||
| > | ||
| <TransparentButton | ||
| onClick={() => handleJbrowseOption(itemLabel, closeDropDownFn)} |
There was a problem hiding this comment.
oh! you went for it in this PR, nice!
this approach is close, but not quite the right solution, as it still creates a new arrow with every render.
I'll open a PR onto this branch to show what I mean.
There was a problem hiding this comment.
ok, not sure how to do this and pass in arguments that aren't attached to the event
There was a problem hiding this comment.
good question! like so https://github.com/overture-stack/dms-ui/pull/188
There was a problem hiding this comment.
decided to leave it as an arrow function due to event typing issues
requires latest (develop) of dms-jbrowse-components