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

Bugs with focusedNodeId #374

Conversation

TranquilMarmot
Copy link
Contributor

@TranquilMarmot TranquilMarmot commented Sep 18, 2020

This is based off of #341 which seems to have been abandoned.

Some bugs I don't know how to fix...

  • onZoomChange prop isn't properly called when zooming in on focused node
    • I tried to add this in .on('end', _zoomEnd) in _zoomConfig but that zoom function isn't called at all when focusing on a node id
    • Putting it in UNSAFE_componentWillReceiveProps caused an infinite loop and a crash
  • Trying to zoom (with the mouse wheel) while a node is focused causes a weird "jump" in the zoom level (the focused node zoom seems to take precedence but only after an update)
  • Trying to pan while there's a focusedNodeId can have unpredictable results; sometimes it will "jump" around and sometimes it won't

@danielcaldas
Copy link
Owner

I'll be merging this one. Thanks for following up on this @TranquilMarmot. I've created a follow-up issue to tackle those bugs you've mentioned: #415

@danielcaldas danielcaldas merged commit b005d2d into danielcaldas:master Dec 18, 2020
@TranquilMarmot TranquilMarmot deleted the tranquilmarmot/fix-focusedNodeId branch February 6, 2021 18:13
TranquilMarmot added a commit to TranquilMarmot/YarnLoom that referenced this pull request Feb 6, 2021
Notable upgrades:

- `react-d3-graph`: `1.5.0` ➡️  `1.6.0`
  - This includes some potential fixes for dragging the viewport after focusing on a node (danielcaldas/react-d3-graph#374)
- `react-scripts` (Create React App): `3.4.1` ➡️  `4.0.1` 
- Move from `@emotion/core` `10.0.28` ➡️  `@emotion/react` `11.1.4`
   - This now requires using [Babel macros](https://emotion.sh/docs/babel-macros)
   - This also required changing `/** @jsx jsx */` to `/** @jsxImportSource @emotion/react */` 🤮 
- `typescript`: `3.9.6` ➡️ `4.1.3`
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.

None yet

2 participants