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.

Sign up for GitHub

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

Jump to bottom

fix: Support Excalidraw inside scrollable container #3018

Merged
merged 8 commits into from
Feb 14, 2021
Merged

fix: Support Excalidraw inside scrollable container #3018

merged 8 commits into from
Feb 14, 2021

Conversation

ad1992
Copy link
Member

@ad1992 ad1992 commented Feb 12, 2021 •
edited
Loading

fixes #3011 & #2737

For host check it out here

TODOS

  • Removed position fixed from everywhere except for context menu since it's rendered inside excalidraw class so top needs to set from the viewport. This will not create any issues in the host as well so should be fine.
  • Account for offsets when the page is scrolled
  • Testing
  • update changelog

@vercel
Copy link

vercel bot commented Feb 12, 2021 •
edited
Loading

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/excalidraw/excalidraw/p8nnczbqf
✅ Preview: https://excalidraw-git-aakansha-fixed.excalidraw.vercel.app

@ad1992 ad1992 marked this pull request as ready for review February 13, 2021 07:47
@ad1992 ad1992 requested review from lipis and dwelle February 13, 2021 07:50
@ad1992 ad1992 changed the title refactor: remove position fixed from excalidraw container, modal and stats refactor: remove position fixed from excalidraw Feb 13, 2021
@dwelle
Copy link
Member

dwelle commented Feb 13, 2021 •
edited by ad1992
Loading

  • fix hitbox testing and everything around scene manipulation

@ad1992
Make excal interactable by fixing offsets and set popover as fixed si…
36b13ae 
…nce position needs to be calculate from viewport top
@ad1992 ad1992 changed the title refactor: remove position fixed from excalidraw feat: Support Excalidraw inside scrollable container Feb 13, 2021
@ad1992 ad1992 changed the title feat: Support Excalidraw inside scrollable container fix: Support Excalidraw inside scrollable container Feb 14, 2021
lipis
lipis reviewed Feb 14, 2021
View reviewed changes
src/components/App.tsx Outdated Show resolved Hide resolved
lipis
lipis reviewed Feb 14, 2021
View reviewed changes
src/components/LayerUI.tsx
className={CLASSES.SHAPE_ACTIONS_MENU}
padding={2}
style={{
maxHeight: `${appState.height - 200}px`,
Copy link
Member

Choose a reason for hiding this comment

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

Another magic number.. what's this 200?

@ad1992 ad1992 merged commit 830fb64 into master Feb 14, 2021
@ad1992 ad1992 deleted the aakansha-fixed branch February 14, 2021 12:48
@ad1992 ad1992 mentioned this pull request Feb 14, 2021
Closed
@ad1992 ad1992 mentioned this pull request Feb 14, 2021
Closed
2 tasks
lipis added a commit that referenced this pull request Feb 14, 2021
@lipis
Merge branch 'master' of github.com:excalidraw/excalidraw into fonts
3fd14b6 
* 'master' of github.com:excalidraw/excalidraw:
  feat: adjust line-confirm-threshold based on zoom (#2884)
  chore: npm audit fix
  fix: Support Excalidraw inside scrollable container (#3018)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lipis lipis lipis approved these changes

@dwelle dwelle dwelle approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support Excalidraw inside scrollable container
3 participants
@ad1992 @dwelle @lipis