Checking that there is indeed a selection before scrolling

[guzmanvig]

Same as before but now I check that the start and end the selection are different. It seems that when you click on some row, the selection prop gets populated with the index you clicked on in start and end

[jjti]

@guzmanvig what is the intended behavior? I'm not sure what's happening now is desirable, clicking on any element, even if it's already in full view of the Linear Viewer, scrolls the Linear viewer so that element is at the top of the viewer. I prefer it the way it is now without that auto-scrolling behavior

[guzmanvig]

@guzmanvig what is the intended behavior? I'm not sure what's happening now is desirable, clicking on any element, even if it's already in full view of the Linear Viewer, scrolls the Linear viewer so that element is at the top of the viewer. I prefer it the way it is now without that auto-scrolling behavior

Screencast.from.02-13-2024.11.51.37.AM.webm

@jjti The intended behavior is for when the user programmatically sets a selection. In our application, the user clicks on a list of indices, which when clicked, they are selected in Seqviz. However, currently, the selection may not be visible and the user needs to manually scroll Seqviz to find it. This would fix that as it would automatically scroll to whatever is selected.

Would you want me to apply this logic only for programatic selection and not for when the user clicks? Or should I check that the selection is not visible before scrolling? Not sure if the latter is possible

[jjti]

Would you want me to apply this logic only for programatic selection and not for when the user clicks?

I think this is the way to go imo. Selections set from clicks on elements differ from the selection that users pass in. They have more fields like type (which is not publicly documented):

seqviz/src/selectionContext.ts

Line 16 in beb9455

export interface Selection {

So maybe check whether that .type field is set and only update state if it is?

[guzmanvig]

@jjti your solution worked, but with one caveat. The selection prop type was just { clockwise?: boolean; end: number; start: number; }, so the type field didn't exist in it. I added an | Selection to the type to fix this, and then a type guard to be able to check for the type field. What I couldn't figure out was why selecting by clicking updates the props. Shouldn't it just update the state? Maybe that's something that's wrong somewhere else, and we need to fix that instead of doing this. Let me know and I can look into it.

[jjti]

I don't think we need to change this one. This is the prop that can only be passed from the public prop. It's the state on the component, and the func args, that need that enhancement

[guzmanvig]

That was my understanding as well, but for some reason the functioncomponentDidUpdate = (prevProps: SeqViewerContainerProps) gets called when a user clicks on an element, and in this.props.selection you see the selection with type and the other fields.

In any case, I removed this since the type guard takes care of checking if it's a Selection or not.

[jjti]

this doesn't need to change either, it's 1:1 w/ the prop that users pass in so it doesn't have those other fields.

Looking thru it quickly, a few lines (maybe more) that need to change:

• state field here: https://github.com/Lattice-Automation/seqviz/blob/develop/src/SeqViewerContainer.tsx#L73
• return type here:

  seqviz/src/SeqViewerContainer.tsx

  Line 126 in 450552d

  getSelection = (

[guzmanvig]

I'm not sure this is right, the selection in the state has to be a Selection because it is then use everywhere else by passing it in <SelectionContext.Provider value={mergedSelection}>. And the getSelection function purpose is to convert the selection in props to a Selection:

  getSelection = (state: Selection, prop?: ExternalSelection): Selection => {
    if (prop) {
      return { ...prop, clockwise: typeof prop.clockwise === "undefined" || !!prop.clockwise, type: "" };
    }
    return state;
  };

I've added an ExternalSelection interface now to make it clearer, let me know what you think.