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.

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

Feat/comments mvi 1 testing #5007

Merged
merged 19 commits into from
Oct 20, 2023

Conversation

skogsmaskin
Copy link
Member

Description

What to review

Notes for release

@vercel
Copy link

vercel bot commented Oct 18, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
performance-studio ✅ Ready (Inspect) Visit Preview Oct 20, 2023 11:39am
test-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 20, 2023 11:39am
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
studio-workshop ⬜️ Ignored (Inspect) Visit Preview Oct 20, 2023 11:39am

@github-actions
Copy link
Contributor

Package Documentation Change
sanity +1%
Full Report
sanity
This branch Next branch
120 documented 119 documented
842 not documented 825 not documented
@sanity/diff
This branch Next branch
13 documented 13 documented
16 not documented 16 not documented
@sanity/block-tools
This branch Next branch
4 documented 4 documented
9 not documented 9 not documented
@sanity/types
This branch Next branch
53 documented 53 documented
233 not documented 233 not documented
sanity/desk
This branch Next branch
83 documented 83 documented
55 not documented 55 not documented
@sanity/portable-text-editor
This branch Next branch
21 documented 21 documented
44 not documented 44 not documented
@sanity/mutator
This branch Next branch
7 documented 7 documented
4 not documented 4 not documented
@sanity/cli
This branch Next branch
1 documented 1 documented
30 not documented 30 not documented
@sanity/schema/_internal
This branch Next branch
0 documented 0 documented
9 not documented 9 not documented
@sanity/util/paths
This branch Next branch
1 documented 1 documented
15 not documented 15 not documented
sanity/router
This branch Next branch
15 documented 15 documented
21 not documented 21 not documented
@sanity/util/legacyDateFormat
This branch Next branch
0 documented 0 documented
3 not documented 3 not documented
@sanity/schema
This branch Next branch
0 documented 0 documented
2 not documented 2 not documented
sanity/cli
This branch Next branch
2 documented 2 documented
0 not documented 0 not documented
@sanity/vision
This branch Next branch
0 documented 0 documented
2 not documented 2 not documented
@sanity/util/fs
This branch Next branch
0 documented 0 documented
3 not documented 3 not documented
sanity/_internal
This branch Next branch
0 documented 0 documented
1 not documented 1 not documented
@sanity/util/content
This branch Next branch
1 documented 1 documented
5 not documented 5 not documented

@github-actions
Copy link
Contributor

github-actions bot commented Oct 18, 2023

Component Testing Report Updated Oct 20, 2023 11:50 AM (UTC)

File Status Duration Passed Skipped Failed
comments/CommentInput.spec.tsx ❌ Failed (Inspect) 17s 8 0 1
formBuilder/ArrayInput.spec.tsx ✅ Passed (Inspect) 4s 3 0 0
formBuilder/inputs/PortableText/Annotations.spec.tsx ✅ Passed (Inspect) 8s 3 0 0
formBuilder/inputs/PortableText/Decorators.spec.tsx ✅ Passed (Inspect) 8s 6 0 0
formBuilder/inputs/PortableText/FocusTracking.spec.tsx ❌ Failed (Inspect) 26s 13 0 2
formBuilder/inputs/PortableText/Input.spec.tsx ✅ Passed (Inspect) 12s 9 0 0
formBuilder/inputs/PortableText/ObjectBlock.spec.tsx ✅ Passed (Inspect) 42s 18 0 0
formBuilder/inputs/PortableText/Styles.spec.tsx ✅ Passed (Inspect) 9s 6 0 0
formBuilder/inputs/PortableText/Toolbar.spec.tsx ✅ Passed (Inspect) 5s 3 0 0

…st condition

* Track editor focus for a focusPath that points to a span's text property.

* Harden test for span text by testing that the _type property is actually a 'span' too.

I found and issue where inline blocks having a .text property also triggered editor focus here, which they shouldn't.
Set the initial editor selection based on the focusPath at mount time.
This case needs to be handled explicitly through a inititalSelection
prop on the Edtiable component of the PTE.
Allow tests to specify their own document and focusPath
Seems like WebKit behaves a bit differently than the other browsers in regard to the initial focusPath,
add method to ensure that the focusPath is propagated before we start to test against it.
'hasFocus' here really means: does it has focus inside the editor itself?
As opposed focus on some object inside the editor content (object blocks etc).
Use children here instead, so tests not testing forms can use it too.
The TestForm will be passed by the different stories instead.
Retry setting focus until the editor has applied all of it's operations.
This will ensure that we don't call focus until the editor has applied
the final selection.

There is currently a bug in slate-react Editable that will cause
errors if we are trying to call ReactEditor.focus in the midst
of a transform.
…ting

This will make sure that the produced Range actually is pointing to content. Error if the Range is invalid.
There was a bug in the delete function that could leave the editor in a broken state
if the last block is removed from the content.
This will fix a issue with the wrong path being sent to the delete function and
focus can now be called without a setTimeout because of a bugfix in the PortableTextEditor.
Add a couple of test-ids we will need when writing tests.
This will fix some bugs we have been seeing related to setting focus in the comment input.
Use the PTE Editable selection prop to control selecting end of the content.
Call focus on the editor without side-effects.
There is no longer a need to re-focus the editor when toggling fullscreen,
so this code can be removed.

Also use a state inside PortableTextEditor to control hasFocusWithin
where the editable itself will control the focus directly.
Add some basic tests for the comment input. More to come.
The toolbar popover should not show when the annotation object is open.
Also make sure it is displayed again when closing the editing popover.
@skogsmaskin skogsmaskin marked this pull request as ready for review October 20, 2023 11:51
@skogsmaskin skogsmaskin merged commit e49d0e2 into feat/comments-mvi-1 Oct 20, 2023
17 checks passed
@skogsmaskin skogsmaskin deleted the feat/comments-mvi-1-testing branch October 20, 2023 11:51
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.

1 participant