Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add infrastructure for creating linear-genome-view sub-classes #3227

Merged
merged 9 commits into from
Sep 29, 2022

Conversation

carolinebridge
Copy link
Contributor

Re-works some components and exposes others such that the linear genome view plugin can be used / manipulated more easily by external plugins (i.e. the multilevel-linear-view.

In this PR:

  • adds the 'extendedName' property to the ViewTypes constructor
    This is required to extend a view type, even when using .named() to extend a view type. This is because when the PluginManager is looping through displays each display has an associated view type. If that view type has already had tracks loaded through it, it ignores the duplicates. An extended view type appears as a duplicate here and without this optional property on view types, the tracks are empty for an extended view, even though it should be able to display all the tracks of its parent / extendee view.
  • modifies some logic to display / not display a header and the minicontrols to better synergize with providing a custom header or custom minicontrols component
  • adds ZoomControls, SearchBox, and LinearGenomeView components to those exported by plugin-linear-genome-view

@cmdcolin
Copy link
Collaborator

cmdcolin commented Sep 28, 2022

I hadn't fully understood the extendedName concept before but I think I do now

My summary of it is: we have, thus far, made it so that "display types" have to manually register themselves with a specific view type. But, the MLLV wants to have in it's tracklist anything the LinearGenomeView has, and it doesn't want to manually try to register "display types" to itself, so it uses "extendedName" to get anything that registers itself as a displayType to LinearGenomeView.

Another name for "extendedName" could perhaps be "includeDisplayTypesFrom": "LinearGenomeView" or something like that, but it's probably ok to stay as is

@@ -75,19 +76,8 @@ const LinearGenomeView = observer(({ model }: { model: LGV }) => {
handleClose={() => model.setSearchResults(undefined, undefined)}
/>
) : null}
{!hideHeader ? (
Copy link
Collaborator

Choose a reason for hiding this comment

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

is there a reason for the mllv why hideHeader got moved into the minicontrols/header?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, MLLV has its own conditions for rendering the header, and if I recall correctly moving these conditions into their own components fixed an issue where the header was being duplicated or not shown at all.

@codecov
Copy link

codecov bot commented Sep 29, 2022

Codecov Report

Merging #3227 (a7b1a50) into main (c6ec3ab) will increase coverage by 0.01%.
The diff coverage is 74.07%.

❗ Current head a7b1a50 differs from pull request most recent head 71ac22a. Consider uploading reports for the commit 71ac22a to get more accurate results

@@            Coverage Diff             @@
##             main    #3227      +/-   ##
==========================================
+ Coverage   59.42%   59.43%   +0.01%     
==========================================
  Files         672      672              
  Lines       28753    28762       +9     
  Branches     6986     6989       +3     
==========================================
+ Hits        17086    17095       +9     
  Misses      11386    11386              
  Partials      281      281              
Impacted Files Coverage Δ
...view/src/LinearGenomeView/components/SearchBox.tsx 95.12% <ø> (ø)
plugins/linear-genome-view/src/index.ts 75.00% <ø> (ø)
...w/src/LinearGenomeView/components/MiniControls.tsx 56.25% <46.15%> (+6.25%) ⬆️
packages/core/PluginManager.ts 92.43% <100.00%> (+0.03%) ⬆️
packages/core/pluggableElementTypes/ViewType.ts 83.33% <100.00%> (+1.51%) ⬆️
...me-view/src/LinearGenomeView/components/Header.tsx 86.36% <100.00%> (+1.36%) ⬆️
...c/LinearGenomeView/components/LinearGenomeView.tsx 81.48% <100.00%> (-0.67%) ⬇️
.../linear-genome-view/src/LinearGenomeView/index.tsx 83.42% <100.00%> (+0.12%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@cmdcolin cmdcolin changed the title Adds infrastructure to the linear genome view for plugin compatibility Adds infrastructure for creating linear-genome-view sub-classes Sep 29, 2022
@cmdcolin cmdcolin merged commit ea9df34 into main Sep 29, 2022
@cmdcolin cmdcolin changed the title Adds infrastructure for creating linear-genome-view sub-classes Add infrastructure for creating linear-genome-view sub-classes Sep 29, 2022
@cmdcolin cmdcolin deleted the mllv-infrastructure branch September 29, 2022 14:40
cmdcolin added a commit that referenced this pull request Sep 30, 2022
* Infrastructure changes necessary for MLLV

* styles import theme and export components runtime

* export for external plugin runtime import

* update snaps

* snaps

* remove ternary and tsignore

snaps

* Update extendedName description

Co-authored-by: Colin <colin.diesh@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants