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

change zoom with (ctrl + mouse wheel) #236

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

Eng-Elias
Copy link
Contributor

make changing zoom easier by using (ctrl + mouse wheel)

@jjti
Copy link
Collaborator

jjti commented Dec 1, 2023

Hey @Eng-Elias! Sorry for slow reply. This looks dope, and I like the idea of supporting this. I hope we can move it into the viewer though. It's unclear from the repo (there's no CONTRIBUTING guide like we should probably have), but your change was made to the demo application and not the viewer itself.

I think you could make a somewhat similar change to the EventHandler here:

export class EventHandler extends React.PureComponent<EventsHandlerProps> {

Maybe with onWheel in the eventRouter at the bottom?

id="la-vz-event-router"

How to percolate that change is unfortunately tricky. You'll need to:

  1. Add zoom to SeqVizState:
    export interface SeqVizState {
  2. Copy zoom from props into state in the constructor:
    this.state = {
  3. Create something like an updateZoom handler on SeqViz to prop drill down into EventHandler.tsx
  4. Pass zoom from state rather than props in the SeqViz render:
    const { highlightedRegions, highlights, primers, showComplement, showIndex, style, zoom } = this.props;
  5. Add the wheel event handler here to EventHandler which then percolates that change (depending on the viewer) up into SeqViz to change its state
  6. (safety in case users change the zoom prop outside SeqViz): Check for whether the zoom prop changes in which case we override the one in state with the one from props:
    { accession = "", annotations, enzymes, enzymesCustom, file, search }: SeqVizProps,

@Eng-Elias
Copy link
Contributor Author

Hi @jjti , sorry for the delay to respond but I moved to Dubai this month and I was very busy.
Thank you for your suggestion, I thought of embedding this feature in the viewer directly but I thought that will not be accepted.
However I will try to work on your suggestion in the next few days and push the changes.

@Eng-Elias
Copy link
Contributor Author

Hi @jjti , I embedded mouse wheel zoom into viewer instead of demo, please review the changes and let me know if you have any note.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants