-
Notifications
You must be signed in to change notification settings - Fork 0
[Resize content][2/n] Implement ResizableSplitView #107
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
Conversation
|
I'll decrease the bar's height tomorrow. @kartikcho I'd appreciate it if you could approve this PR first so that I can land everything tomorrow morning :) |
| } | ||
|
|
||
| _getResizeBarDesiredSize(): Size { | ||
| return nullthrows( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it really necessary to introduce a dependency for a simple helper function? I think we can just write a function to accommodate for it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can definitely declare it ourselves, but since this nullthrows package is used in the React monorepo as well I figured it's okay to just add it as a dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that makes sense.
Summary --- Implements `ResizableSplitView` and uses it to allow resizing of the React measures view. This PR also fixes the last PR's known issue where the flamechart does not scroll vertically. Implements #19. Implements #70. Test Plan --- * `yarn lint` * `yarn flow`: no errors in changed code * `yarn test`: no added tests, but everything still passes * `yarn start`: manual testing: * Hover over resize bar to observe color change * Drag resize bar to resize * Resize bar position is clamped and cannot go offscreen or expand beyond the size of the lanes area * Resize bar remains visible on page resize * Content is independently vertically scrollable There are still 3 independent horizontal pan/zoom areas, a known issue carried over from the last PR in this stack. This will be fixed in the next PR.
3cf668c to
e2b1bfd
Compare
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/mlh-fellowship/scheduling-profiler-prototype/48r57zsok |

Stack PR by STACK ATTACK:
Summary
Implements
ResizableSplitViewand uses it to allow resizing of theReact measures view.
This PR also fixes the last PR's known issue where the flamechart does
not scroll vertically.
Resolves #19, resolves #70.
Test Plan
yarn lintyarn flow: no errors in changed codeyarn test: no added tests, but everything still passesyarn start: manual testing:beyond the size of the lanes area
There are still 3 independent horizontal pan/zoom areas, a known issue
carried over from the last PR in this stack. This will be fixed in the
next PR (#108).