From fd0ed8f8f8d453520104e813a76710433b3c2c8a Mon Sep 17 00:00:00 2001 From: Shashank Budhanuru Ramaraju Date: Tue, 8 Oct 2024 14:25:22 +0100 Subject: [PATCH 1/5] config options for track graphical and table views --- .../components/LinearApolloDisplay.tsx | 4 -- .../LinearApolloDisplay/stateModel/base.ts | 60 ++++++++++++++++ .../src/makeDisplayComponent.tsx | 69 ++++++++++++------- 3 files changed, 104 insertions(+), 29 deletions(-) diff --git a/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/components/LinearApolloDisplay.tsx b/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/components/LinearApolloDisplay.tsx index 8670d80dc..88b4100e4 100644 --- a/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/components/LinearApolloDisplay.tsx +++ b/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/components/LinearApolloDisplay.tsx @@ -64,7 +64,6 @@ export const LinearApolloDisplay = observer(function LinearApolloDisplay( setSeqTrackCanvas, setSeqTrackOverlayCanvas, setTheme, - tabularEditor, } = model const { classes } = useStyles() const lgv = getContainingView(model) as unknown as LinearGenomeViewModel @@ -170,9 +169,6 @@ export const LinearApolloDisplay = observer(function LinearApolloDisplay( onMouseLeave={onMouseLeave} onMouseDown={onMouseDown} onMouseUp={onMouseUp} - onClick={() => { - tabularEditor.showPane() - }} className={classes.canvas} style={{ cursor: cursor ?? 'default' }} data-testid="overlayCanvas" diff --git a/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/stateModel/base.ts b/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/stateModel/base.ts index 86ff4b6a7..a96b163c4 100644 --- a/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/stateModel/base.ts +++ b/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/stateModel/base.ts @@ -41,6 +41,8 @@ export function baseModelFactory( .props({ type: types.literal('LinearApolloDisplay'), configuration: ConfigurationReference(configSchema), + graphical: true, + table: false, }) .volatile((self) => ({ lgv: getContainingView(self) as unknown as LinearGenomeViewModel, @@ -118,7 +120,65 @@ export function baseModelFactory( return (self.session as unknown as ApolloSessionModel) .apolloSelectedFeature }, + get showGraphical() { + return self.graphical + }, + get showTable() { + return self.table + }, + })) + .actions((self) => ({ + showGraphicalOnly() { + self.graphical = true + self.table = false + }, + showTableOnly() { + self.graphical = false + self.table = true + }, + showGraphicalAndTable() { + self.graphical = true + self.table = true + }, })) + .views((self) => { + const { trackMenuItems: superTrackMenuItems } = self + + return { + trackMenuItems() { + return [ + ...superTrackMenuItems(), + { + label: 'Show graphical display', + type: 'radio', + // eslint-disable-next-line unicorn/consistent-destructuring + checked: self.graphical && !self.table, + onClick: () => { + self.showGraphicalOnly() + }, + }, + { + label: 'Show table display', + type: 'radio', + // eslint-disable-next-line unicorn/consistent-destructuring + checked: self.table && !self.graphical, + onClick: () => { + self.showTableOnly() + }, + }, + { + label: 'Show both graphical and table display', + type: 'radio', + // eslint-disable-next-line unicorn/consistent-destructuring + checked: self.table && self.graphical, + onClick: () => { + self.showGraphicalAndTable() + }, + }, + ] + }, + } + }) .actions((self) => ({ setSelectedFeature(feature?: AnnotationFeature) { ;( diff --git a/packages/jbrowse-plugin-apollo/src/makeDisplayComponent.tsx b/packages/jbrowse-plugin-apollo/src/makeDisplayComponent.tsx index 883156434..2fa02e302 100644 --- a/packages/jbrowse-plugin-apollo/src/makeDisplayComponent.tsx +++ b/packages/jbrowse-plugin-apollo/src/makeDisplayComponent.tsx @@ -161,13 +161,24 @@ export const DisplayComponent = observer(function DisplayComponent({ height: overallHeight, isShown, selectedFeature, + showGraphical, + showTable, tabularEditor, toggleShown, } = model - const detailsHeight = tabularEditor.isShown ? model.detailsHeight : 0 - const featureAreaHeight = isShown - ? overallHeight - detailsHeight - accordionControlHeight * 2 - : 0 + let detailsHeight, featureAreaHeight + if (showGraphical && !showTable) { + featureAreaHeight = overallHeight + } + if (showTable && !showGraphical) { + detailsHeight = overallHeight + } + if (showGraphical && showTable) { + detailsHeight = tabularEditor.isShown ? model.detailsHeight : 0 + featureAreaHeight = isShown + ? overallHeight - detailsHeight - accordionControlHeight * 2 + : 0 + } const onDetailsResize = (delta: number) => { model.setDetailsHeight(model.detailsHeight - delta) @@ -179,27 +190,35 @@ export const DisplayComponent = observer(function DisplayComponent({ }, [model, selectedFeature]) return (
- -
- -
- -
- -
+ {showGraphical ? ( + <> + +
+ +
+ + ) : null} + {showTable ? ( + <> + +
+ +
+ + ) : null}
) }) From 1b5f940e603f581562c3f028c42ede98250cb4d2 Mon Sep 17 00:00:00 2001 From: Shashank Budhanuru Ramaraju Date: Tue, 8 Oct 2024 15:42:30 +0100 Subject: [PATCH 2/5] enable both graphical and table by default --- .../src/LinearApolloDisplay/stateModel/base.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/stateModel/base.ts b/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/stateModel/base.ts index a96b163c4..5c80c34a1 100644 --- a/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/stateModel/base.ts +++ b/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/stateModel/base.ts @@ -42,7 +42,7 @@ export function baseModelFactory( type: types.literal('LinearApolloDisplay'), configuration: ConfigurationReference(configSchema), graphical: true, - table: false, + table: true, }) .volatile((self) => ({ lgv: getContainingView(self) as unknown as LinearGenomeViewModel, From 7227fbde3d5ff9ed619e0aa7a5e9eb32d0047f42 Mon Sep 17 00:00:00 2001 From: Garrett Stevens Date: Fri, 18 Oct 2024 21:56:19 +0000 Subject: [PATCH 3/5] Remove unused config from LinearApolloDisplay --- .../src/LinearApolloDisplay/configSchema.ts | 19 +++++-------------- .../src/LinearApolloDisplay/index.ts | 2 +- .../stateModel/trackHeightMixin.ts | 4 +--- packages/jbrowse-plugin-apollo/src/index.ts | 4 ++-- 4 files changed, 9 insertions(+), 20 deletions(-) diff --git a/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/configSchema.ts b/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/configSchema.ts index 2b4c33ee7..ea1bc8154 100644 --- a/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/configSchema.ts +++ b/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/configSchema.ts @@ -1,16 +1,7 @@ import { ConfigurationSchema } from '@jbrowse/core/configuration' -import PluginManager from '@jbrowse/core/PluginManager' -import type LinearGenomeViewPlugin from '@jbrowse/plugin-linear-genome-view' -export function configSchemaFactory(pluginManager: PluginManager) { - const LGVPlugin = pluginManager.getPlugin( - 'LinearGenomeViewPlugin', - ) as LinearGenomeViewPlugin - const { baseLinearDisplayConfigSchema } = LGVPlugin.exports - - return ConfigurationSchema( - 'LinearApolloDisplay', - { height: { type: 'number', defaultValue: 500 } }, - { baseConfiguration: baseLinearDisplayConfigSchema, explicitlyTyped: true }, - ) -} +export const configSchema = ConfigurationSchema( + 'LinearApolloDisplay', + {}, + { explicitIdentifier: 'displayId', explicitlyTyped: true }, +) diff --git a/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/index.ts b/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/index.ts index 52f85f4fa..b2c570386 100644 --- a/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/index.ts +++ b/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/index.ts @@ -1,2 +1,2 @@ -export { configSchemaFactory } from './configSchema' +export { configSchema } from './configSchema' export { stateModelFactory } from './stateModel' diff --git a/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/stateModel/trackHeightMixin.ts b/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/stateModel/trackHeightMixin.ts index 447f8106e..a7e4196a6 100644 --- a/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/stateModel/trackHeightMixin.ts +++ b/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/stateModel/trackHeightMixin.ts @@ -1,5 +1,4 @@ // TODO: get this added to LGV runtime exports so we don't have to duplicate it -import { getConf } from '@jbrowse/core/configuration' import { types } from 'mobx-state-tree' const minDisplayHeight = 20 @@ -23,8 +22,7 @@ export const TrackHeightMixin = types })) .views((self) => ({ get height() { - // @ts-expect-error getConf needs self.configuration - return self.heightPreConfig ?? (getConf(self, 'height') as number) + return self.heightPreConfig ?? 500 }, })) .actions((self) => ({ diff --git a/packages/jbrowse-plugin-apollo/src/index.ts b/packages/jbrowse-plugin-apollo/src/index.ts index 1f60d81fd..849ff5739 100644 --- a/packages/jbrowse-plugin-apollo/src/index.ts +++ b/packages/jbrowse-plugin-apollo/src/index.ts @@ -64,7 +64,7 @@ import { } from './FeatureDetailsWidget' import { stateModelFactory as LinearApolloDisplayStateModelFactory, - configSchemaFactory as linearApolloDisplayConfigSchemaFactory, + configSchema as linearApolloDisplayConfigSchema, } from './LinearApolloDisplay' import { DisplayComponent, @@ -174,7 +174,7 @@ export default class ApolloPlugin extends Plugin { }) pluginManager.addDisplayType(() => { - const configSchema = linearApolloDisplayConfigSchemaFactory(pluginManager) + const configSchema = linearApolloDisplayConfigSchema return new DisplayType({ name: 'LinearApolloDisplay', configSchema, From 98ecb28748a64eaa360a6587d85c96017bacf2b1 Mon Sep 17 00:00:00 2001 From: Garrett Stevens Date: Fri, 18 Oct 2024 21:57:01 +0000 Subject: [PATCH 4/5] Move table/graphical options to submenu --- .../LinearApolloDisplay/stateModel/base.ts | 119 +++++++++++------- .../stateModel/trackHeightMixin.ts | 41 ------ .../src/makeDisplayComponent.tsx | 97 +++++++------- 3 files changed, 123 insertions(+), 134 deletions(-) delete mode 100644 packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/stateModel/trackHeightMixin.ts diff --git a/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/stateModel/base.ts b/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/stateModel/base.ts index 5c80c34a1..a6ba8bb73 100644 --- a/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/stateModel/base.ts +++ b/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/stateModel/base.ts @@ -22,31 +22,27 @@ import { addDisposer, getRoot, types } from 'mobx-state-tree' import { ApolloInternetAccountModel } from '../../ApolloInternetAccount/model' import { ApolloSessionModel } from '../../session' import { ApolloRootModel } from '../../types' -import { TrackHeightMixin } from './trackHeightMixin' + +const minDisplayHeight = 20 export function baseModelFactory( _pluginManager: PluginManager, configSchema: AnyConfigurationSchemaType, ) { - // TODO: Restore this when TRackHeightMixin is in LGV runtime exports - - // const LGVPlugin = pluginManager.getPlugin( - // 'LinearGenomeViewPlugin', - // ) as LinearGenomeViewPlugin - // const { TrackHeightMixin } = LGVPlugin.exports - - return types - .compose(BaseDisplay, TrackHeightMixin) - .named('BaseLinearApolloDisplay') + return BaseDisplay.named('BaseLinearApolloDisplay') .props({ type: types.literal('LinearApolloDisplay'), configuration: ConfigurationReference(configSchema), graphical: true, - table: true, + table: false, + heightPreConfig: types.maybe( + types.refinement( + 'displayHeight', + types.number, + (n) => n >= minDisplayHeight, + ), + ), }) - .volatile((self) => ({ - lgv: getContainingView(self) as unknown as LinearGenomeViewModel, - })) .views((self) => { const { configuration, renderProps: superRenderProps } = self return { @@ -59,6 +55,26 @@ export function baseModelFactory( }, } }) + .volatile(() => ({ + scrollTop: 0, + })) + .views((self) => ({ + get lgv() { + return getContainingView(self) as unknown as LinearGenomeViewModel + }, + get height() { + if (self.heightPreConfig) { + return self.heightPreConfig + } + if (self.graphical && self.table) { + return 500 + } + if (self.graphical) { + return 200 + } + return 300 + }, + })) .views((self) => ({ get rendererTypeName() { return self.configuration.renderer.type @@ -120,14 +136,20 @@ export function baseModelFactory( return (self.session as unknown as ApolloSessionModel) .apolloSelectedFeature }, - get showGraphical() { - return self.graphical - }, - get showTable() { - return self.table - }, })) .actions((self) => ({ + setScrollTop(scrollTop: number) { + self.scrollTop = scrollTop + }, + setHeight(displayHeight: number) { + self.heightPreConfig = Math.max(displayHeight, minDisplayHeight) + return self.height + }, + resizeHeight(distance: number) { + const oldHeight = self.height + const newHeight = this.setHeight(self.height + distance) + return newHeight - oldHeight + }, showGraphicalOnly() { self.graphical = true self.table = false @@ -143,37 +165,40 @@ export function baseModelFactory( })) .views((self) => { const { trackMenuItems: superTrackMenuItems } = self - return { trackMenuItems() { + const { graphical, table } = self return [ ...superTrackMenuItems(), { - label: 'Show graphical display', - type: 'radio', - // eslint-disable-next-line unicorn/consistent-destructuring - checked: self.graphical && !self.table, - onClick: () => { - self.showGraphicalOnly() - }, - }, - { - label: 'Show table display', - type: 'radio', - // eslint-disable-next-line unicorn/consistent-destructuring - checked: self.table && !self.graphical, - onClick: () => { - self.showTableOnly() - }, - }, - { - label: 'Show both graphical and table display', - type: 'radio', - // eslint-disable-next-line unicorn/consistent-destructuring - checked: self.table && self.graphical, - onClick: () => { - self.showGraphicalAndTable() - }, + type: 'subMenu', + label: 'Appearance', + subMenu: [ + { + label: 'Show graphical display', + type: 'radio', + checked: graphical && !table, + onClick: () => { + self.showGraphicalOnly() + }, + }, + { + label: 'Show table display', + type: 'radio', + checked: table && !graphical, + onClick: () => { + self.showTableOnly() + }, + }, + { + label: 'Show both graphical and table display', + type: 'radio', + checked: table && graphical, + onClick: () => { + self.showGraphicalAndTable() + }, + }, + ], }, ] }, diff --git a/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/stateModel/trackHeightMixin.ts b/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/stateModel/trackHeightMixin.ts deleted file mode 100644 index a7e4196a6..000000000 --- a/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/stateModel/trackHeightMixin.ts +++ /dev/null @@ -1,41 +0,0 @@ -// TODO: get this added to LGV runtime exports so we don't have to duplicate it -import { types } from 'mobx-state-tree' - -const minDisplayHeight = 20 - -/** - * #stateModel TrackHeightMixin - * #category display - */ -export const TrackHeightMixin = types - .model({ - heightPreConfig: types.maybe( - types.refinement( - 'displayHeight', - types.number, - (n) => n >= minDisplayHeight, - ), - ), - }) - .volatile(() => ({ - scrollTop: 0, - })) - .views((self) => ({ - get height() { - return self.heightPreConfig ?? 500 - }, - })) - .actions((self) => ({ - setScrollTop(scrollTop: number) { - self.scrollTop = scrollTop - }, - setHeight(displayHeight: number) { - self.heightPreConfig = Math.max(displayHeight, minDisplayHeight) - return self.height - }, - resizeHeight(distance: number) { - const oldHeight = self.height - const newHeight = this.setHeight(self.height + distance) - return newHeight - oldHeight - }, - })) diff --git a/packages/jbrowse-plugin-apollo/src/makeDisplayComponent.tsx b/packages/jbrowse-plugin-apollo/src/makeDisplayComponent.tsx index 2fa02e302..117e30fcd 100644 --- a/packages/jbrowse-plugin-apollo/src/makeDisplayComponent.tsx +++ b/packages/jbrowse-plugin-apollo/src/makeDisplayComponent.tsx @@ -158,67 +158,72 @@ export const DisplayComponent = observer(function DisplayComponent({ const { classes } = useStyles() const { + detailsHeight, + graphical, height: overallHeight, isShown, selectedFeature, - showGraphical, - showTable, + table, tabularEditor, toggleShown, } = model - let detailsHeight, featureAreaHeight - if (showGraphical && !showTable) { - featureAreaHeight = overallHeight - } - if (showTable && !showGraphical) { - detailsHeight = overallHeight + + const canvasScrollContainerRef = useRef(null) + useEffect(() => { + scrollSelectedFeatureIntoView(model, canvasScrollContainerRef) + }, [model, selectedFeature]) + + const onDetailsResize = (delta: number) => { + model.setDetailsHeight(detailsHeight - delta) } - if (showGraphical && showTable) { - detailsHeight = tabularEditor.isShown ? model.detailsHeight : 0 - featureAreaHeight = isShown + + if (graphical && table) { + const tabularHeight = tabularEditor.isShown ? detailsHeight : 0 + const featureAreaHeight = isShown ? overallHeight - detailsHeight - accordionControlHeight * 2 : 0 + return ( +
+ +
+ +
+ +
+ +
+
+ ) } - const onDetailsResize = (delta: number) => { - model.setDetailsHeight(model.detailsHeight - delta) + if (graphical) { + return ( +
+ +
+ ) } - const canvasScrollContainerRef = useRef(null) - useEffect(() => { - scrollSelectedFeatureIntoView(model, canvasScrollContainerRef) - }, [model, selectedFeature]) return (
- {showGraphical ? ( - <> - -
- -
- - ) : null} - {showTable ? ( - <> - -
- -
- - ) : null} +
) }) From 397f01c4486c88d1eb17c98851fcb24ad14a9c3f Mon Sep 17 00:00:00 2001 From: Garrett Stevens Date: Fri, 18 Oct 2024 22:53:40 +0000 Subject: [PATCH 5/5] Fix tests --- .../cypress/e2e/editFeature.cy.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/jbrowse-plugin-apollo/cypress/e2e/editFeature.cy.ts b/packages/jbrowse-plugin-apollo/cypress/e2e/editFeature.cy.ts index 594e5d3a8..5a25a3d66 100644 --- a/packages/jbrowse-plugin-apollo/cypress/e2e/editFeature.cy.ts +++ b/packages/jbrowse-plugin-apollo/cypress/e2e/editFeature.cy.ts @@ -34,6 +34,9 @@ describe('Different ways of editing features', () => { .within(() => { cy.get('[data-testid="CloseIcon"]').click() }) + cy.get('[data-testid="track_menu_icon"]').click() + cy.contains('Appearance').trigger('mouseover') + cy.contains('Show both graphical and table display').click() cy.contains('Table') .parent() @@ -82,6 +85,9 @@ describe('Different ways of editing features', () => { cy.addAssemblyFromGff('onegene.fasta.gff3', 'test_data/onegene.fasta.gff3') cy.selectAssemblyToView('onegene.fasta.gff3') cy.searchFeatures('gx1', 1) + cy.get('[data-testid="track_menu_icon"]').click() + cy.contains('Appearance').trigger('mouseover') + cy.contains('Show both graphical and table display').click() cy.contains('td', 'CDS1').rightclick() cy.contains('Edit attributes').click() cy.contains('Feature attributes') @@ -108,6 +114,9 @@ describe('Different ways of editing features', () => { cy.addAssemblyFromGff('stopcodon', 'test_data/cdsChecks/stopcodon.gff3') cy.selectAssemblyToView('stopcodon') cy.searchFeatures('gene02', 1) + cy.get('[data-testid="track_menu_icon"]').click() + cy.contains('Appearance').trigger('mouseover') + cy.contains('Show both graphical and table display').click() cy.contains('td', '=cds02.1').rightclick() cy.contains('Delete feature').click() cy.contains('Are you sure you want to delete the selected feature?') @@ -126,6 +135,9 @@ describe('Different ways of editing features', () => { cy.addAssemblyFromGff('stopcodon', 'test_data/cdsChecks/stopcodon.gff3') cy.selectAssemblyToView('stopcodon') cy.searchFeatures('gene04', 1) + cy.get('[data-testid="track_menu_icon"]').click() + cy.contains('Appearance').trigger('mouseover') + cy.contains('Show both graphical and table display').click() cy.contains('td', '=cds04.1').rightclick() cy.contains('Delete feature').click() @@ -145,6 +157,9 @@ describe('Different ways of editing features', () => { cy.addAssemblyFromGff('onegene.fasta.gff3', 'test_data/onegene.fasta.gff3') cy.selectAssemblyToView('onegene.fasta.gff3') cy.searchFeatures('gx1', 1) + cy.get('[data-testid="track_menu_icon"]').click() + cy.contains('Appearance').trigger('mouseover') + cy.contains('Show both graphical and table display').click() cy.contains('td', '=CDS1') cy.contains('td', '=tx1').rightclick() cy.contains('Delete feature').click() @@ -163,6 +178,9 @@ describe('Different ways of editing features', () => { cy.addAssemblyFromGff('onegene.fasta.gff3', 'test_data/onegene.fasta.gff3') cy.selectAssemblyToView('onegene.fasta.gff3') cy.searchFeatures('gx1', 1) + cy.get('[data-testid="track_menu_icon"]').click() + cy.contains('Appearance').trigger('mouseover') + cy.contains('Show both graphical and table display').click() // In headless mode it seems to take a long time for menus to be populated cy.get('input[type="text"][value="CDS"]', { timeout: 60_000 }).click({ timeout: 60_000, @@ -181,6 +199,9 @@ describe('Different ways of editing features', () => { cy.addAssemblyFromGff('onegene.fasta.gff3', 'test_data/onegene.fasta.gff3') cy.selectAssemblyToView('onegene.fasta.gff3') cy.searchFeatures('gx1', 1) + cy.get('[data-testid="track_menu_icon"]').click() + cy.contains('Appearance').trigger('mouseover') + cy.contains('Show both graphical and table display').click() // In headless mode it seems to take a long time for menus to be populated cy.get('input[type="text"][value="CDS"]', { timeout: 60_000 }).rightclick({ timeout: 60_000,