Adds session model reference to view in focus

[carolinebridge]

Resolves #2174

• Adds a root session model property "focusedViewId" that permits reference to the view in focus
• Stores the id of the view
• The focused view is such when a user clicks or tabs to the view
• The focused view is also updated when a user selects a widget associated with the view
• There is a visual indication (on the background of the view and widget) of which view is in focus, and whether the opened widget is associated with it

[carolinebridge]

Note: omitted work on bookmarks discussed in July 26th backlog grooming to make this PR more on-topic. What's been done here doesn't actually violate the current behaviour of the bookmarks.

[carolinebridge]

Solicited feedback:

1. Is the illustrated visual indication (horizontal stripes) what we want for the “highlight” of the focused view?
   • note that in discussion, we have identified that an “aura” or an icon indicator is probably not doable/appropriate due to screen real estate limitations
2. How do we feel about the focused View being updated with the currently opened Widget (that is associated with a view?)
   • As opposed to maintaining which view is in focus based on where the user clicked or tabbed to last

• anything else :)

[codecov]

Codecov Report

Merging #3831 (2068f42) into main (130aa25) will increase coverage by 0.04%.
Report is 8 commits behind head on main.
The diff coverage is 82.08%.

@@            Coverage Diff             @@
##             main    #3831      +/-   ##
==========================================
+ Coverage   64.35%   64.40%   +0.04%     
==========================================
  Files         998      999       +1     
  Lines       29656    29722      +66     
  Branches     7116     7137      +21     
==========================================
+ Hits        19084    19141      +57     
- Misses      10411    10419       +8     
- Partials      161      162       +1     

Files Changed	Coverage Δ
packages/app-core/src/ui/App/App.tsx	96.55% <ø> (ø)
packages/app-core/src/ui/App/DrawerWidget.tsx	69.23% <ø> (ø)
packages/core/util/types/index.ts	63.55% <ø> (ø)
packages/web-core/src/BaseWebSession/index.ts	73.04% <ø> (ø)
products/jbrowse-desktop/src/rootModel/index.ts	47.05% <ø> (ø)
products/jbrowse-desktop/src/sessionModel/index.ts	40.54% <ø> (ø)
products/jbrowse-web/src/rootModel/rootModel.ts	35.36% <ø> (ø)
products/jbrowse-web/src/sessionModel/index.ts	50.00% <ø> (ø)
packages/app-core/src/AppFocus/index.ts	40.00% <40.00%> (ø)
...w/src/LinearGenomeView/components/MiniControls.tsx	75.00% <60.00%> (-8.34%)	⬇️
... and 4 more

... and 3 files with indirect coverage changes

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

[carolinebridge]

Some UI feedback from Lincoln:

The angled lines look too much like "caution tape" (e.g. something might be 'wrong' with the view)...he suggested making the in-focus view more 'saturated' and the out of focus views lesso.

Some concepts:
1

infocus: theme.palette.secondary.light
outfocus: theme.palette.secondary.dark

2

infocus: adding an 'interior' outline of theme.quaternary.main

3

infocus: theme.palette.quaternary.main

[cmdcolin]

it is likely unneeded to put ref in dependency array https://epicreact.dev/why-you-shouldnt-put-refs-in-a-dependency-array/

[carolinebridge]

I included it in the dependency array because of a lint warning: 76:6 warning React Hook useEffect has missing dependencies: 'ref'. Either include it or remove the dependency array react-hooks/exhaustive-deps

Which that article does mention:

The reason we get the warning there when we didn't get it within the component is because ESLint is pretty limited in its ability to trace what you're doing with your JavaScript. So the React plugin for ESLint can't know that cbRef is actually a ref. So just to be safe, it warns you.

Luckily, as we just learned, including it in the dependency array doesn't make any difference anyway, so just include it!

Let me know if maybe I'm reading that wrong.

[cmdcolin]

technically there is the concept of "subviews", so a synteny view contains two linear genome view's each with their own track selectors. the concept of the focused view in this case may not work because it only looks at session.views

[carolinebridge]

From UI discussions aug 9:

Light/dark with outline:

widget unfocused:

Widget unassociated with a view is palette.secondary.main:

For comparison, Light/dark without outline:

Multi view concept:

^ subview in focus

^ main view in focus

[carolinebridge]

@cmdcolin the lint issue is that the focus mixin had to be added to both react linear genome view and circular view to satisfy the changes to the AbstractSessionModel to include the new properties for focus on the session -- the linter wants app-core to be added to their dependencies but I don't want to needlessly inflate those projects. Thoughts?

[cmdcolin]

probably don't want to add app-core to the embedded widgets

the abstractsessionmodel can make those properties optional to satisfy this, or a abstractsessionmodel subtype "SessionModelWithFocusedView" could be created (there are several types like this in core)

[cmdcolin]

i think it looks great at https://jbrowse.org/code/jb2/add-focus-view/?session=share-jty6tZYS58&password=GvT5P

could probably merge? any reason why the dimensions have changed in some of the snaps?

[carolinebridge]

Trying to diagnose the snaps now, no idea what's causing it >:(

[cmdcolin]

merged post v2.6.3 release, can do a little usability testing on it via main :)