OnChangePlugin ignoreInitialChange -> ignoreHistoryMergeTagChange #2706
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zurfyx
requested review from
trueadm,
fantactuka,
acywatson,
tylerjbainbridge and
thegreatercurve
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
facebook-github-bot
added
the
CLA Signed
|
Why do we even need these props? I thought the |
thegreatercurve
approved these changes
Jul 26, 2022
Yes, when |
karam-qaoud
pushed a commit
to karam-qaoud/lexical
that referenced
this pull request
Jul 26, 2022
…cebook#2706) * OnChangePlugin ignoreInitialChange -> ignoreHistoryMergeTag * . * default to false because 0.4
karam-qaoud
added a commit
that referenced
this pull request
Jul 26, 2022
* Fixed getStyleObjectFromRawCSS function to work for unformatted css strings * Testing that handles unformatted css text * Testing that $getStyleObjectFromRawCss handles unformatted css * Added TableOfContents * Renamed TableOfContetnsPlugin file name and added flow file * Added TableOfContentsPlugin to config files and added styling * Fixed types * Added TableOfContentsList as a seperate module * Fixed type of tag from string to HeadingTagType * Table of contents updates as user scrolls * Wrapped plugin in a feature * Deleted package-lock.json * Fixed conditioanl rendering syntax * Removed extra parameter * package-lock * fix imports * Update packages/lexical-playground/src/plugins/TableOfContentsPlugin.tsx Co-authored-by: Gerard Rovira <zurfyx@users.noreply.github.com> * Added sticky styling and handled text overflow * Table of contents updates automtaically on scroll without observing all heading nodes * Update table correctly when headings are not visible but exist either up or down * Fix failing E2E * Changed isTableOfContentes to showTableOfContents in settings * Added useEffect to fix memory leak * Hoisted functions that don't use props * Renamed isTableOfContets to showTableOfContents * Changing selectedHeading by observing page top * resolved lint error * Refactored scroll up logic * Added comments * Added better css * Changed place of toc div to fix failing test * Fixed adjacent headings scrolling * Fixed adjacent headings bug * Renamed helper methods * Fixed test * Added dependency array to useEffect * Added TableOfContents to dependency array * Updated dependeny array in useEffect * Created e2e test file for table of contents plugin * Added scroll test * E2E test: Adding heading to editor adds them to table-of-contents * clean up * Refactored getEditorElement * Scrolling callback has better conditions * Table of contents is now covering all edge cases and doesn't freeze webpage * Solved page freezing * Added one more assert statment to second test * chore(lexical-playground): make directory clear (#2674) * Conditionally utilize `startTransition` if it's present (#2676) * Only utilize startTransition if it's available * Add type annotation * Run prettier * fix(lexical-list): remove list breaks if selection in empty (#2672) * fix(lexical-list): remove list breaks if selection in empty * chore: add a comment * chore: add test * Separate `@lexical/code` into more atomic modules (#2673) * separate code package into more atomic modules * remove utils * named exports * Fixed typo (#2678) * fix: path to icons (#2683) * Fix VALID_TWITTER_URL to allow underscores. (#2690) * fix(lexical-playground): LexicalTypeaheadMenuPlugin import (#2689) Use the correct import path that available in NPM package * Collapse and Expand DevTools Tree Nodes (#2679) * fix(playground): fix rendering Exclidraw (#2694) * Make includeHeaders a boolean (#2697) Changed type for includeHeaders parameter from string to boolean to match the type of the parameter from the $createTableNodeWithDimensions function. * Remove coverage reports (#2699) * fix: check if options are empty (#2701) * feat: Link node with target and rel (#2687) * OnChangePlugin ignoreInitialChange -> ignoreHistoryMergeTagChange (#2706) * OnChangePlugin ignoreInitialChange -> ignoreHistoryMergeTag * . * default to false because 0.4 Co-authored-by: Karam Qaoud <kqaoud@fb.com> Co-authored-by: Gerard Rovira <zurfyx@users.noreply.github.com> Co-authored-by: 子瞻 Luci <haru.lucinyan@gmail.com> Co-authored-by: Jack Hanford <jackhanford@gmail.com> Co-authored-by: John Flockton <thegreatercurve@users.noreply.github.com> Co-authored-by: SalvadorLekan <66782276+SalvadorLekan@users.noreply.github.com> Co-authored-by: Adithya Vardhan <imadithyavardhan@gmail.com> Co-authored-by: hiraoka <62982380+y-hiraoka@users.noreply.github.com> Co-authored-by: Elvin Dzhavadov <elvin.d@outlook.com> Co-authored-by: Will <will.gutierrez@gmail.com> Co-authored-by: Bryan <ImSingee@users.noreply.github.com> Co-authored-by: alinamusuroi <44519061+alinamusuroi@users.noreply.github.com> Co-authored-by: Andriy Chemerynskiy <andrzej.chem@gmail.com>
thegreatercurve
added a commit
that referenced
this pull request
Nov 25, 2022
* Fixed getStyleObjectFromRawCSS function to work for unformatted css strings * Testing that handles unformatted css text * Testing that $getStyleObjectFromRawCss handles unformatted css * Added TableOfContents * Renamed TableOfContetnsPlugin file name and added flow file * Added TableOfContentsPlugin to config files and added styling * Fixed types * Added TableOfContentsList as a seperate module * Fixed type of tag from string to HeadingTagType * Table of contents updates as user scrolls * Wrapped plugin in a feature * Deleted package-lock.json * Fixed conditioanl rendering syntax * Removed extra parameter * package-lock * fix imports * Update packages/lexical-playground/src/plugins/TableOfContentsPlugin.tsx Co-authored-by: Gerard Rovira <zurfyx@users.noreply.github.com> * Added sticky styling and handled text overflow * Table of contents updates automtaically on scroll without observing all heading nodes * Update table correctly when headings are not visible but exist either up or down * Fix failing E2E * Changed isTableOfContentes to showTableOfContents in settings * Added useEffect to fix memory leak * Hoisted functions that don't use props * Renamed isTableOfContets to showTableOfContents * Changing selectedHeading by observing page top * resolved lint error * Refactored scroll up logic * Added comments * Added better css * Changed place of toc div to fix failing test * Fixed adjacent headings scrolling * Fixed adjacent headings bug * Renamed helper methods * Fixed test * Added dependency array to useEffect * Added TableOfContents to dependency array * Updated dependeny array in useEffect * Created e2e test file for table of contents plugin * Added scroll test * E2E test: Adding heading to editor adds them to table-of-contents * clean up * Refactored getEditorElement * Scrolling callback has better conditions * Table of contents is now covering all edge cases and doesn't freeze webpage * Solved page freezing * Added one more assert statment to second test * chore(lexical-playground): make directory clear (#2674) * Conditionally utilize `startTransition` if it's present (#2676) * Only utilize startTransition if it's available * Add type annotation * Run prettier * fix(lexical-list): remove list breaks if selection in empty (#2672) * fix(lexical-list): remove list breaks if selection in empty * chore: add a comment * chore: add test * Separate `@lexical/code` into more atomic modules (#2673) * separate code package into more atomic modules * remove utils * named exports * Fixed typo (#2678) * fix: path to icons (#2683) * Fix VALID_TWITTER_URL to allow underscores. (#2690) * fix(lexical-playground): LexicalTypeaheadMenuPlugin import (#2689) Use the correct import path that available in NPM package * Collapse and Expand DevTools Tree Nodes (#2679) * fix(playground): fix rendering Exclidraw (#2694) * Make includeHeaders a boolean (#2697) Changed type for includeHeaders parameter from string to boolean to match the type of the parameter from the $createTableNodeWithDimensions function. * Remove coverage reports (#2699) * fix: check if options are empty (#2701) * feat: Link node with target and rel (#2687) * OnChangePlugin ignoreInitialChange -> ignoreHistoryMergeTagChange (#2706) * OnChangePlugin ignoreInitialChange -> ignoreHistoryMergeTag * . * default to false because 0.4 Co-authored-by: Karam Qaoud <kqaoud@fb.com> Co-authored-by: Gerard Rovira <zurfyx@users.noreply.github.com> Co-authored-by: 子瞻 Luci <haru.lucinyan@gmail.com> Co-authored-by: Jack Hanford <jackhanford@gmail.com> Co-authored-by: John Flockton <thegreatercurve@users.noreply.github.com> Co-authored-by: SalvadorLekan <66782276+SalvadorLekan@users.noreply.github.com> Co-authored-by: Adithya Vardhan <imadithyavardhan@gmail.com> Co-authored-by: hiraoka <62982380+y-hiraoka@users.noreply.github.com> Co-authored-by: Elvin Dzhavadov <elvin.d@outlook.com> Co-authored-by: Will <will.gutierrez@gmail.com> Co-authored-by: Bryan <ImSingee@users.noreply.github.com> Co-authored-by: alinamusuroi <44519061+alinamusuroi@users.noreply.github.com> Co-authored-by: Andriy Chemerynskiy <andrzej.chem@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
ignoreInitialChangeproperty is a trap. While, it's logically well implemented it doesn't account for plugins that may load later in time and cause nodes to be dirty even when this results to 0 DOM changes.This race condition is accentuated when we use
editorStateinside LexicalComposer instead as it loads even before otheruseLayoutEffectsHence, I'm proposing killing this property and replacing it for
ignoreHistoryMergeTagthat practically speaking does the same job we intended this one to do.