If you load a shared scene with at least another person on the scene, you can start seeing the content via the firebase response. If you scroll and you receive the response from the websocket INIT, then it scrolls you back to the center which is jarring.
This PR removes the scroll to content for that use case.
My last attempt removed the gesture handler altogether but broke macbooks. This one keeps it working on macbook but makes sure it zooms properly on iPhone and iPad.
If you start the gesture on the chrome, gesture is not defined and the zoom will be set to NaN and you will have to refresh.
Typescript was actually right and we shouldn't have overridden the bang ;)
Repro:
- On iPhone touch down on a shape, touch down on the chrome, start moving around
- See that the selection is off, but the zoom is not being modified like crazy.
https://user-images.githubusercontent.com/197597/102722206-6c182400-42b4-11eb-9865-6ae1cd0af9be.MP4
I already fixed this but a special case for no elements was added in getCommonBounds to return 0 and reintroduce this bug. I'm not exactly sure where to put this check tbh. Fixing it here so that I'm not annoyed anymore at least.
I checked some of the callsites, some of them related to selection will never pass an empty array, some to export will break if we remove the 0, 0, 0, 0 fix.
For rendering we always use mouse in order to check which handles to display but when doing the hit test, we used pointer which has a different size. So we couldn't use the middle handles for small shapes. This is now fixed.
cc @j-f1 as you added it in #790
We were processing both the touch move and gesture on iOS which was first firing twice as many set state, but also caused issues:
- The gesture implementation didn't support zooming on the center
- Touching down on a circle and then on the bottom chrome would freak out because initialScale was null
Touching down on the menu still isn't perfect as it shifts the shape around but doesn't completly break the zoom
This was very annoying when you would zoom on mobile and the context menu would appear.
The problem was the following:
- You put a finger in, it creates a timeout
- You put a second finger in, it creates another timeout
- 300ms elapsed, which is not that much
- The context menu opens
- Now you move your fingers, which works, but the context menu is still open
The fix is to invalidate the context menu if a second finger is added even if the first one hasn't moved.
This was an oversight to enable it for all the shapes. I don't believe that we want to be able to insert text on double click when drawing a rectangle for example. And it's definitely a broken experience when doing so for free draw.
Fixes part of #1935
Fixes#1115
The issue is that replaceAllElements calls a render synchronously, preventing lastBroadcastedOrReceivedSceneVersion from being set correctly.
I tried using batchUpdate but it only takes a single argument ( https://github.com/facebook/react/blob/c5d2fc7127654e43de59fff865b74765a103c4a5/packages/react-reconciler/src/ReactFiberWorkLoop.js#L1088 ) whereas the callback takes two.
Test Plan:
- Add a console.log before `this.broadcastScene("SCENE_UPDATE");` in App.tsx
- Connect a bunch of clients
- Have one move a shape
- Make sure that this client has the console logged
- Make sure the other clients don't have it
The logic to support it was not implemented. This is not the prettiest way to solve it in the world but it does work. Some more refactoring here is probably warranted.
Fixes#1039
* Don't reset cache while zooming using a gesture
This reuses the cached canvas while the gesture is happening. Once it has stop updating, then recompute the cache with the proper zoom.
This should massively improve performance when panning on big scenes on mobile
Fixes#1056
* update snapshot tests
This wasn't taking into account zoom properly.
The logic should probably get refactored a bit, it's not ideal that we're passing canvas, state and scale as different arguments. Also it's weird that the function that returns the center is computing the viewport translation. But I'm not motivated enough to fix it right now...
Fixes#1100
* Fix hit testing threshold
- The bounding box was not correctly extended to take into account the threshold. It was only for y axis but not x.
- The bezier threshold was using 20 instead of 10 and not taking into account zoom level.
Both those issues are fixed and now the behavior looks good on all the shapes I can test.
* fix_tests
My original hack to put the scale when we create the canvas element doesn't make much sense. It should be done when we are rendering the scene. I moved it there in this PR.
The rest was all about forwarding the scale to where it's needed.
* Stop using getTransform
Fixes#861
The original motivation behind this is to make it work with Firefox. But it also helped make the code more intentional.
Test Plan:
- Create one square, select it, zoom in repeatedly, make sure that it zooms centered in the screen and everything looks good
- Scroll at various zoom levels, things look good
- Export a small scene at 1x and 3x, make sure the background is properly set and look good
* fix selection element
I'm using the client id as a random number to index on the color array. So far it's been working better than using a sequential increment as the colors in the array are sorted by proximity. Also, it has the advantage that everyone in the room will see the same color for the same person.
The computation was not correct. I'm not really sure how it used to work but it was not taking into account the dimensions of the scene so it was wrong.
The new algorithm is computing the scrollbar such that it's the position of the viewport in relationship to the bounding box of the viewport and all the elements.
Fixes#680
* End to end encryption description
This PR updates the url upload description to mention that it is end to end encrypted. I used a very similar message as whatsapp so that it is familar to people.
I also removed the automatic copying and turned the alert into prompt. This should be less awkward than the current implementation.
* capitalize excalidraw
Co-authored-by: David Luzar <luzar.david@gmail.com>
* Add encryption
In order to avoid the server being able to read the content of the scene, this PR implements local encryption and decryption. This implements the algorithm described in #610.
Right now the server doesn't support uploading binary files. I mocked the server with comments. @lipis, could you add support on the server and update this PR? I added a bunch of TODO: that tell you where to comment/uncomment in order to get the server flow going.
To test locally right now:
- Import: Open http://localhost:3000/#json=1234,5oYVOnGpWYPPTz19-PMYYw and see a square
- Export: Click the export link and see the right url with the private key + the encrypted binary in the console
Fixes#610
* backend_v2
* v2
* Add keybindings for shapes
I'm not 100% sure about this one. I feel like it's going to help people be a lot more productive to display the key bindings at all time. But it also clutters the UI...
* increase font-size
* fix shape keybindings for non-qwerty keyboards
* tweak position and color
Co-authored-by: David Luzar <luzar.david@gmail.com>
With the infinite scroll behavior, it's easy to scroll super far away from where the content is and have a hard time getting back. This PR adds a button to refocus on the center of the scene when no elements are visible anymore.
* Add keybindings for color picker
This adds the ability to navigate using left/right/bottom/up keys and shows key bindings for all the different colors. This is only optimized for the qwerty keyboard layout, but unfortunately it's not possible to detect other keyboard layouts :(
* add aria-keyshortcuts and keybinding in title
* make focus select color, confirm on enter
Co-authored-by: David Luzar <luzar.david@gmail.com>
Turns out the root cause was the outline. For some reason, doing "transparent" doesn't work but doing "1px solid transparent" does. Don't know why but I'll take it!
There's a bug where the carret doesn't show up when the text is first focused on Chrome with the previous combination of CSS. I tweaked it and now it seems to work (don't ask me why!).
Unfortunately on Safari, ever since we moved to contentEditable on #274, the carret disappeared the first time. I unsuccessfully tried to repro in a smaller codebase ( https://jsfiddle.net/u2mjs90y/1/ ) but it does work in Safari...
I'm not exactly sure what's going on, there are bunch of issues opened against this bug when googling against all the browsers...
We did some hackery to prevent copy pasting when we didn't support multilines. But we do now so we can remove it.
Interestingly, newlines are actually <br /> elements. So we need to tweak the isInputLike logic a bit
Thanks to this stack overflow answer ( https://stackoverflow.com/a/41678350/232122 ) I was able to fix the font preloading!
If we put fonts.css in the public/folder and include it with normal html, we can avoid going through the build pipeline!
By using position: fixed like the rest of the UI components, it will no longer make the body change size and have bad side effects like scrolling.
Fixes#365
This is the only way to navigate using the keyboard and was prevented by #596. Now it works fine.
Test Plan:
- Click on selection icon
- Use left/right arrow keys to move between tool icons, that works.
- Click on a shape, cmd+c
- Click on the selection icon
- Cmd+v, it pastes correctly
The test that was added is not a good test. If we want to test things, we should be testing logic that is error prone such as all the mouse handling logic and state management. Adding a test for something trivial as displaying a list of data is just going to be annoying when we eventually change the UI and the test breaks.
Since this is the only test using enzyme, I also removed enzyme. We can add it back if we want to test a component using it.
There are two problems with the current localization strategy:
- We download the translations on-demand, which means that it does a serial roundtrip for nothing.
- withTranslation helper actually renders the app 3 times on startup, instead of once (I haven't tried to debug it)
Instead of finding all the places where we want to resume recording, we should do it after every componentDidUpdate(). The idea is that we just want to disable the history for certain setState, for which we call directly before skipHistory.
Some PR made the settings UI show up even though nothing is selected. However, you couldn't actually change any of those settings except for the colors. This PR pipes through the rest of the properties so everything works now!
When we are scrolling, resizing, or moving elements, we already disable the history. Since those actions do not change the state of the UI, we can also avoid re-drawing it and save ~10ms per frame.
I had to change all the forceUpdate() to setState({}), otherwise it would bypass shouldComponentUpdate.
* Add a gap between shapes and lock
The lock is a different type as the rest of the shapes, so we should visually separate it.
* redesign lock icon
Co-authored-by: David Luzar <luzar.david@gmail.com>
* Only show correct settings
The logic to display which settings when nothing was selected was incorrect. This PR ensures that they are in sync.
I also removed all the <hr /> which after the redesigned just looked like weird empty spaces
* fix handling editing/text elements
Co-authored-by: David Luzar <luzar.david@gmail.com>
A common workflow I have is to enable the lock, draw a bunch of things, unlock to be able to select stuff. However, after I unlock, the last shape is still enabled, so I end up drawing yet another of the same shape :(
This PR resets to selection instead!
If you have two elements selected that have a different value (eg: a green line and a red line), the value of the color picker should be undefined, not the default value.
If you scroll and draw to the left of the origin, when you export the scene, there's a weird whitespace on the right. This is because we do the min() computation starting at 0 and not -Infinity
This also fixes pasted elements and scrollbars.
This updates the way fonts are handled to not have to download the font from the internet.
If you do `./public/font.ttf` in the .scss file, unfortunately the bundle packages them somewhere that's different from the public folder that is used by the index.html to preload them.
The fix I used is to use absolute path `/font.ttf` so that both work. Unfortunately, this means that the website will only work if at the root of the domain. That's the case so far so it's okay but still annoying if we want to embed it somewhere.
Problem:
- Select arrow
- Mouse down somewhere
- Mouse move somewhere to create an arrow
- Press escape
- Now you're in a weird situation where the shape is now "selection" but you're still dragging the arrow. If you mouse up, the arrow disappears
In order to solve this problem, we can avoid making escape do anything if you're currently dragging an element
Sorry my OCD is kicking in... It's super weird that the base of the lock moves when we check / unckeck it. Instead, just the semi-circle shape should move (what this PR implements).
It turns out the only thing we need to save in the appState is the background color. All the rest is transient data.
I added `"type": "excalidraw"` at the beginning to explain where it was.
I removed `"source": "http://localhost:3000/"`. I don't think we want to leak on which webpage it was saved from.
I removed `isSelected` from the json
I added indentation so it's easier to read the content. I'm not 100% sure on this one, but I figure filesize doesn't matter too much those days. And if we want to shrink it, there are more effective ways than json.
Right now we're running useEffect block which runs getExportCanvasPreview on every state update, even if the modal is not visible (eg: when moving the mouse around!). This puts the modal code in its own component so that it doesn't trigger useEffect when not visible.
The code isn't very elegant as we're forwarding all the props, there's likely a better way to handle it (if anyone is interested, PR would be appreciated), but at least now it no longer double renders the scene.
Fixes#559
* Do not store cursor position in state
Storing it in state causes a full re-render. The only time we use the cursor position is for pasting. This halves the number of renders on drag.
* remove passive change
Fixes#252
Test plan:
- Click on text icon
- Click anywhere to start entering text
- Add a letter
- Make sure the cursor is selection and not text
- Click anywhere else, make sure it completes the text and not create a new one
I profiled dragging and it looks like it takes ~3ms to save to localStorage a smallish scene and we're doing it twice per mousemove. Let's debounce so we don't pay that cost on every mouse move.
Stole the implementation from #220 which got reverted.
Unfortunately, react-color has a bug where transparent color doesn't trigger onChange. I've been annoyed by the huge dependency anyway so decided to take the generated html (which is awesome) and reimplement a specific component for it.
I also made sure that we don't actually render anything when the background is transparent on rough (I looked at the generated path and made sure it didn't have the commands for the background)
- Move the context menu right next to the mouse so it's not so far away. But 1px out so that nothing is selected until you move your mouse
- Change the colors to be closer to the macos one. Unfortunately, macos has a 0.5px border that I'm not able to reproduce without some annoying hacks, 1px it'll be.
