-
Notifications
You must be signed in to change notification settings - Fork 4.8k
RTC: Fix inline inserter reset on update sync #76980
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
Changes from all commits
143a0ae
e828339
4559810
1dfa3b1
7d47aa3
2a2611a
de9c87d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -294,17 +294,41 @@ export function useAutocomplete( { | |
| }; | ||
| } | ||
|
|
||
| function useLastDifferentValue( value: UseAutocompleteProps[ 'record' ] ) { | ||
| const history = useRef< Set< typeof value > >( new Set() ); | ||
| /** | ||
| * Checks whether two records represent the same user-visible state | ||
| * (same text content and cursor position). | ||
| */ | ||
| function recordValuesMatch( | ||
| a: UseAutocompleteProps[ 'record' ], | ||
| b: UseAutocompleteProps[ 'record' ] | ||
| ) { | ||
| return a.text === b.text && a.start === b.start && a.end === b.end; | ||
| } | ||
|
Comment on lines
+297
to
+306
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess this technically can produce a false positive: same text, different formats. Though probably not important for this case. P.S. Seems like the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right that this misses some of the text record attributes. I considered adding them all, but Comparing text + start/end will let us know if the last |
||
|
|
||
| history.current.add( value ); | ||
| /** | ||
| * Tracks the last record whose value differed from the current one. | ||
| * Used to determine whether the user has actually typed something | ||
| */ | ||
|
Comment on lines
+308
to
+311
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can simplify the JSDoc block here. There's no need to mention previous methods or anything else. Just what it does currently.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed most of the lines in 7d47aa3! |
||
| export function useLastDifferentValue( | ||
| value: UseAutocompleteProps[ 'record' ] | ||
| ) { | ||
| const history = useRef< Array< typeof value > >( [] ); | ||
|
|
||
| const lastEntry = history.current[ history.current.length - 1 ]; | ||
|
|
||
| // Only add to history if the value is meaningfully different from | ||
| // the most recent entry (analogous to Set.add being a no-op for | ||
| // duplicate references in the original implementation). | ||
| if ( ! lastEntry || ! recordValuesMatch( value, lastEntry ) ) { | ||
| history.current.push( value ); | ||
| } | ||
|
|
||
| // Keep the history size to 2. | ||
| if ( history.current.size > 2 ) { | ||
| history.current.delete( Array.from( history.current )[ 0 ] ); | ||
| if ( history.current.length > 2 ) { | ||
| history.current.shift(); | ||
| } | ||
|
|
||
| return Array.from( history.current )[ 0 ]; | ||
| return history.current[ 0 ]; | ||
| } | ||
|
|
||
| export function useAutocompleteProps( options: UseAutocompleteProps ) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changelog here has git conflict markers. I'm removing them at #77127
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be fixed on trunk.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oandregal Thank you for fixing this, I'm not sure how this got by me.