undo textarea #2030
undo textarea #2030
Conversation
filipesilva
commented
Feb 18, 2022
filipesilva
commented
- rfct: add a few e2e utils
- feat: use the same undo for local and remote
- test: add undo e2e
- fix: always use athens undo/redo
- rfct: remove old single player undo
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/athens-research/athens/GGrKQFtS6vG5HKJYAmEEVBfLKMBg |
filipesilva
force-pushed
the
undo-textarea
branch
from
b96bc83
to
7f51c40
Compare
| @@ -1,212 +0,0 @@ | |||
| (ns athens.common-events.left-sidebar-test | |||
There was a problem hiding this comment.
I think we refactored these events into atomic events, disabled the test here, and never deleted it. I only found it because I removed the .resolve ns due to removing undo.
| await page.keyboard.press(undoShortcut); | ||
|
|
||
| // Last block should be gone. | ||
| await expect(page.locator('.block:has-text("one")')).toBeVisible(); |
There was a problem hiding this comment.
How is expect async here? I thought this would have to be written as
expect(await page.locator('.block:has-text("one")')).toBeVisible();
There was a problem hiding this comment.
I think the expectation itself is async, because I know it can wait a certain amount of time for it to be or not be visible. If the locator itself was the async part here, I don't think it could wait to be visible (e.g. it would return the locator immediately and then the expectation could only tell if it was visible or not at that point).
|
|
||
| export const goToDailyPages = async (page:Page) => { | ||
| // The sixth button is the daily notes button. | ||
| // TODO: find a better way to address this button, maybe tooltip? |
There was a problem hiding this comment.
I think we could CSS classes to the Typescript components and select them that way.
There was a problem hiding this comment.
Usually for e2e we're looking at things a user could see in some way. A CSS class doesn't fit the bill, but a particular icon, text, or tooltip text does. If all else fails though, it'd have to be a CSS class or ID.
| // Wait for an element that signals the app has finished loading. | ||
| // Normally on e2e tests we'd load a page, but on a electron app we should rely | ||
| // only on visible behaviour. | ||
| // TODO: This isn't necessary on production builds, but is necessary for dev |
|
What's needed to get |
|
Also, if user undos after creating and navigating to a page, they are left with an empty page. Should they go back to their previous page? Should a user be able to undo content that is on another page, even if they aren't on that page when they undo? There's a more general problem here of how to undo/redo not just datascript state but also DOM state:
Seems hard :) https://twitter.com/steveruizok/status/1495317652046290944 |
|
We probably need some guardrails around large undo effects, stuff like deleting a whole page or moving like 40 blocks. Think that's out of scope of this PR though. Unsure what concretely we need to do to set the cursor on the right place, but I am sure it's doable with what we have right now. We need to have a stronger concept of "block to focus", and save it on the undo/redo stack. |
