search

Lattice-Automation / seqviz

a JavaScript DNA, RNA, and protein sequence viewer
https://tools.latticeautomation.com/seqviz
MIT License
247 stars 53 forks source link

change zoom with (ctrl + mouse wheel) #236

Open Eng-Elias opened 1 year ago

Eng-Elias commented 1 year ago

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

jjti commented 12 months ago

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: https://github.com/Lattice-Automation/seqviz/blob/ea558f2e2dd74ac19fa7e5dd03215246a2b05695/src/EventHandler.tsx#L21

Maybe with onWheel in the eventRouter at the bottom? https://github.com/Lattice-Automation/seqviz/blob/ea558f2e2dd74ac19fa7e5dd03215246a2b05695/src/EventHandler.tsx#L253

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

  1. Add zoom to SeqVizState: https://github.com/Lattice-Automation/seqviz/blob/ea558f2e2dd74ac19fa7e5dd03215246a2b05695/src/SeqViz.tsx#L172
  2. Copy zoom from props into state in the constructor: https://github.com/Lattice-Automation/seqviz/blob/ea558f2e2dd74ac19fa7e5dd03215246a2b05695/src/SeqViz.tsx#L217
  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: https://github.com/Lattice-Automation/seqviz/blob/ea558f2e2dd74ac19fa7e5dd03215246a2b05695/src/SeqViz.tsx#L420
  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: https://github.com/Lattice-Automation/seqviz/blob/ea558f2e2dd74ac19fa7e5dd03215246a2b05695/src/SeqViz.tsx#L281
Eng-Elias commented 11 months ago

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 commented 11 months ago

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.