Skip to content

Conversation

@taneliang
Copy link
Member

@taneliang taneliang commented Aug 3, 2020

Stack PR by STACK ATTACK:

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.

Resolves #19, resolves #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:
    image
    • Hover over resize bar to observe color change
    • Drag resize bar to resize
      image
    • Resize bar position is clamped and cannot go offscreen or expand
      beyond the size of the lanes area
    • Content is independently vertically scrollable
      image
    • Resize bar remains visible on page resize
      image

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).

@taneliang
Copy link
Member Author

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(
Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that makes sense.

Base automatically changed from sttack-laudantium-temporibus-sint to master August 4, 2020 05:07
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.
@taneliang taneliang force-pushed the sttack-architecto-cumque-vitae branch from 3cf668c to e2b1bfd Compare August 4, 2020 05:10
@vercel
Copy link

vercel bot commented Aug 4, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/mlh-fellowship/scheduling-profiler-prototype/48r57zsok
✅ Preview: https://scheduling-profiler-prototyp-git-sttack-architecto-cumqu-10b758.mlh-fellowship.vercel.app

@taneliang
Copy link
Member Author

Decreased bar height by 1px (it's now 5px tall) and rebased on master

image

@taneliang taneliang merged commit fcc9af4 into master Aug 4, 2020
@taneliang taneliang deleted the sttack-architecto-cumque-vitae branch August 4, 2020 05:12
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.

Make lanes section independently vertically scrollable Add vertical resizer between canvas

2 participants