From e0d9069131a83bac4ff5734dda753a8d71e25035 Mon Sep 17 00:00:00 2001 From: Alf Eaton Date: Fri, 28 Apr 2023 10:37:36 +0100 Subject: [PATCH] [cm6] Add "within selection" option to the search form (#12798) * Use forked @codemirror/search * Use getPanel to get the search panel * Remove layer-based highlightSelectionMatches * Add "within selection" option to the search form * Add test for "replace all within selection" * Fix tests GitOrigin-RevId: 95ce76fd017f96278b04c16a1fd34f785f7504a3 --- package-lock.json | 15 +- .../web/frontend/extracted-translations.json | 1 + .../components/codemirror-search-form.tsx | 48 ++++++- .../components/codemirror-search.tsx | 8 +- .../extensions/highlight-selection-matches.ts | 128 ------------------ .../source-editor/extensions/search.ts | 107 +++++++++++++-- .../source-editor/extensions/theme.ts | 14 -- services/web/locales/en.json | 1 + services/web/package.json | 2 +- .../components/codemirror-editor.spec.tsx | 23 ++++ 10 files changed, 178 insertions(+), 169 deletions(-) delete mode 100644 services/web/frontend/js/features/source-editor/extensions/highlight-selection-matches.ts diff --git a/package-lock.json b/package-lock.json index d244f9de58..8cefd92078 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3249,8 +3249,9 @@ }, "node_modules/@codemirror/search": { "version": "6.4.0", - "resolved": "https://registry.npmjs.org/@codemirror/search/-/search-6.4.0.tgz", - "integrity": "sha512-zMDgaBXah+nMLK2dHz9GdCnGbQu+oaGRXS1qviqNZkvOCv/whp5XZFyoikLp/23PM9RBcbuKUUISUmQHM1eRHw==", + "resolved": "git+ssh://git@github.com/overleaf/codemirror-search.git#ea83364b22ad66455fc94babea7d576fa9f76a93", + "integrity": "sha512-02UOFSNY7/FamUaRPNPwcjq58V2nsRbtXRIT85/AKgfEWv6tVHj5slobCeaRKCXb6hPSOEDMztR5ShmbuxLfEw==", + "license": "MIT", "dependencies": { "@codemirror/state": "^6.0.0", "@codemirror/view": "^6.0.0", @@ -35172,7 +35173,7 @@ "@codemirror/lang-markdown": "^6.1.1", "@codemirror/language": "^6.6.0", "@codemirror/lint": "^6.2.1", - "@codemirror/search": "^6.4.0", + "@codemirror/search": "github:overleaf/codemirror-search#ea83364b22ad66455fc94babea7d576fa9f76a93", "@codemirror/state": "^6.2.0", "@codemirror/view": "^6.9.6", "@contentful/rich-text-html-renderer": "^16.0.2", @@ -40025,9 +40026,9 @@ } }, "@codemirror/search": { - "version": "6.4.0", - "resolved": "https://registry.npmjs.org/@codemirror/search/-/search-6.4.0.tgz", - "integrity": "sha512-zMDgaBXah+nMLK2dHz9GdCnGbQu+oaGRXS1qviqNZkvOCv/whp5XZFyoikLp/23PM9RBcbuKUUISUmQHM1eRHw==", + "version": "git+ssh://git@github.com/overleaf/codemirror-search.git#ea83364b22ad66455fc94babea7d576fa9f76a93", + "integrity": "sha512-02UOFSNY7/FamUaRPNPwcjq58V2nsRbtXRIT85/AKgfEWv6tVHj5slobCeaRKCXb6hPSOEDMztR5ShmbuxLfEw==", + "from": "@codemirror/search@github:overleaf/codemirror-search#ea83364b22ad66455fc94babea7d576fa9f76a93", "requires": { "@codemirror/state": "^6.0.0", "@codemirror/view": "^6.0.0", @@ -44852,7 +44853,7 @@ "@codemirror/lang-markdown": "^6.1.1", "@codemirror/language": "^6.6.0", "@codemirror/lint": "^6.2.1", - "@codemirror/search": "^6.4.0", + "@codemirror/search": "github:overleaf/codemirror-search#ea83364b22ad66455fc94babea7d576fa9f76a93", "@codemirror/state": "^6.2.0", "@codemirror/view": "^6.9.6", "@contentful/rich-text-html-renderer": "^16.0.2", diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index 8c93d082a9..db0c9c69ab 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -748,6 +748,7 @@ "search_replace_with": "", "search_search_for": "", "search_whole_word": "", + "search_within_selection": "", "select_a_file": "", "select_a_payment_method": "", "select_a_project": "", diff --git a/services/web/frontend/js/features/source-editor/components/codemirror-search-form.tsx b/services/web/frontend/js/features/source-editor/components/codemirror-search-form.tsx index e90ecb765e..6424c48f2f 100644 --- a/services/web/frontend/js/features/source-editor/components/codemirror-search-form.tsx +++ b/services/web/frontend/js/features/source-editor/components/codemirror-search-form.tsx @@ -21,10 +21,16 @@ import Tooltip from '../../../shared/components/tooltip' import Icon from '../../../shared/components/icon' import classnames from 'classnames' import useScopeValue from '../../../shared/hooks/use-scope-value' +import { getStoredSelection, setStoredSelection } from '../extensions/search' const MAX_MATCH_COUNT = 1000 -type ActiveSearchOption = 'caseSensitive' | 'regexp' | 'wholeWord' | null +type ActiveSearchOption = + | 'caseSensitive' + | 'regexp' + | 'wholeWord' + | 'withinSelection' + | null const CodeMirrorSearchForm: FC = () => { const view = useCodeMirrorViewContext() @@ -42,6 +48,7 @@ const CodeMirrorSearchForm: FC = () => { const caseSensitiveId = 'caseSensitive' + idSuffix const regexpId = 'regexp' + idSuffix const wholeWordId = 'wholeWord' + idSuffix + const withinSelectionId = 'withinSelection' + idSuffix const { t } = useTranslation() @@ -114,12 +121,19 @@ const CodeMirrorSearchForm: FC = () => { regexp: data.regexp === 'on', literal: true, wholeWord: data.wholeWord === 'on', + scope: getStoredSelection(view.state)?.ranges, }) view.dispatch({ effects: setSearchQuery.of(query) }) } }, [view]) + const handleWithinSelectionChange = useCallback(() => { + const storedSelection = getStoredSelection(state) + view.dispatch(setStoredSelection(storedSelection ? null : state.selection)) + handleChange() + }, [handleChange, state, view]) + const handleFormKeyDown = useCallback( event => { if (runScopeHandlers(view, event, 'search-panel')) { @@ -308,6 +322,27 @@ const CodeMirrorSearchForm: FC = () => { + + + + + + { onFocus={() => setActiveSearchOption('wholeWord')} onBlur={() => setActiveSearchOption(null)} /> + + setActiveSearchOption('withinSelection')} + onBlur={() => setActiveSearchOption(null)} + />
diff --git a/services/web/frontend/js/features/source-editor/components/codemirror-search.tsx b/services/web/frontend/js/features/source-editor/components/codemirror-search.tsx index a533348518..9eda1fcdd1 100644 --- a/services/web/frontend/js/features/source-editor/components/codemirror-search.tsx +++ b/services/web/frontend/js/features/source-editor/components/codemirror-search.tsx @@ -1,17 +1,19 @@ import { createPortal } from 'react-dom' import CodeMirrorSearchForm from './codemirror-search-form' import { useCodeMirrorViewContext } from './codemirror-editor' +import { getPanel } from '@codemirror/view' +import { createSearchPanel } from '@codemirror/search' function CodeMirrorSearch() { const view = useCodeMirrorViewContext() - const dom = view.dom.querySelector('.ol-cm-search') + const panel = getPanel(view, createSearchPanel) - if (!dom) { + if (!panel) { return null } - return createPortal(, dom) + return createPortal(, panel.dom) } export default CodeMirrorSearch diff --git a/services/web/frontend/js/features/source-editor/extensions/highlight-selection-matches.ts b/services/web/frontend/js/features/source-editor/extensions/highlight-selection-matches.ts deleted file mode 100644 index 5272d15b1e..0000000000 --- a/services/web/frontend/js/features/source-editor/extensions/highlight-selection-matches.ts +++ /dev/null @@ -1,128 +0,0 @@ -/** - * This file is adapted from CodeMirror 6, licensed under the MIT license: - * https://github.com/codemirror/search/blob/main/src/selection-match.ts - */ -import { EditorView, layer, RectangleMarker } from '@codemirror/view' -import { - CharCategory, - EditorSelection, - EditorState, - Extension, -} from '@codemirror/state' -import { SearchCursor } from '@codemirror/search' -import { rectangleMarkerForRange } from '../utils/layer' - -/* -This extension highlights text that matches the selection. -It uses the `"cm-selectionMatch"` class for the highlighting. - */ -export const highlightSelectionMatches = (): Extension => [ - layer({ - above: false, - markers(view) { - return buildMarkers(view, view.state) - }, - update(update) { - return update.docChanged || update.selectionSet || update.viewportChanged - }, - class: 'ol-cm-selectionMatchesLayer', - }), - EditorView.baseTheme({ - '.ol-cm-selectionMatchesLayer': { - contain: 'size style', - pointerEvents: 'none', - }, - '.cm-selectionMatch': { - position: 'absolute', - }, - }), -] - -// Whether the characters directly outside the given positions are non-word characters -function insideWordBoundaries( - check: (char: string) => CharCategory, - state: EditorState, - from: number, - to: number -): boolean { - return ( - (from === 0 || - check(state.sliceDoc(from - 1, from)) !== CharCategory.Word) && - (to === state.doc.length || - check(state.sliceDoc(to, to + 1)) !== CharCategory.Word) - ) -} - -// Whether the characters directly at the given positions are word characters -function insideWord( - check: (char: string) => CharCategory, - state: EditorState, - from: number, - to: number -): boolean { - return ( - check(state.sliceDoc(from, from + 1)) === CharCategory.Word && - check(state.sliceDoc(to - 1, to)) === CharCategory.Word - ) -} - -const buildMarkers = ( - view: EditorView, - state: EditorState -): RectangleMarker[] => { - const sel = state.selection - if (sel.ranges.length > 1) { - return [] - } - - const range = sel.main - - if (range.empty) { - return [] - } - - const len = range.to - range.from - if (len < 3 || len > 200) { - return [] - } - - const query = state.sliceDoc(range.from, range.to) // TODO: allow and include leading/trailing space? - if (query === '') { - return [] - } - - const check = state.charCategorizer(range.head) - if ( - !( - insideWordBoundaries(check, state, range.from, range.to) && - insideWord(check, state, range.from, range.to) - ) - ) { - return [] - } - - const markers: RectangleMarker[] = [] - - for (const part of view.visibleRanges) { - const cursor = new SearchCursor(state.doc, query, part.from, part.to) - - while (!cursor.next().done) { - const { from, to } = cursor.value - - if (!check || insideWordBoundaries(check, state, from, to)) { - markers.push( - ...rectangleMarkerForRange( - view, - 'cm-selectionMatch', - EditorSelection.range(from, to) - ) - ) - - if (markers.length > 100) { - return [] - } - } - } - } - return markers -} diff --git a/services/web/frontend/js/features/source-editor/extensions/search.ts b/services/web/frontend/js/features/source-editor/extensions/search.ts index 2e3eaeebd2..cf8f84a0e4 100644 --- a/services/web/frontend/js/features/source-editor/extensions/search.ts +++ b/services/web/frontend/js/features/source-editor/extensions/search.ts @@ -1,29 +1,28 @@ import { - searchKeymap, search as searchExtension, setSearchQuery, getSearchQuery, openSearchPanel, SearchQuery, searchPanelOpen, + searchKeymap, + highlightSelectionMatches, + togglePanel, } from '@codemirror/search' -import { EditorView, keymap, ViewPlugin } from '@codemirror/view' +import { Decoration, EditorView, keymap, ViewPlugin } from '@codemirror/view' import { Annotation, + Compartment, + EditorSelection, EditorState, SelectionRange, + StateEffect, + StateField, TransactionSpec, } from '@codemirror/state' -import { highlightSelectionMatches } from './highlight-selection-matches' const restoreSearchQueryAnnotation = Annotation.define() -const ignoredSearchKeybindings = new Set([ - // This keybinding causes issues with entering @ on certain keyboard layouts - // https://github.com/overleaf/internal/issues/12119 - 'Alt-g', -]) - const selectNextMatch = (query: SearchQuery, state: EditorState) => { if (!query.valid) { return false @@ -41,6 +40,69 @@ const selectNextMatch = (query: SearchQuery, state: EditorState) => { return result.done ? null : result.value } +const storedSelectionEffect = StateEffect.define() + +const storedSelectionState = StateField.define({ + create() { + return null + }, + update(value, tr) { + if (value) { + value = value.map(tr.changes) + } + + for (const effect of tr.effects) { + if (effect.is(storedSelectionEffect)) { + value = effect.value + } else if (effect.is(togglePanel) && effect.value === false) { + value = null // clear the stored selection when closing the search panel + } + } + + return value + }, + provide(f) { + return [ + EditorView.decorations.from(f, selection => { + if (!selection) { + return Decoration.none + } + const decorations = selection.ranges + .filter(range => !range.empty) + .map(range => + Decoration.mark({ + class: 'ol-cm-stored-selection', + }).range(range.from, range.to) + ) + return Decoration.set(decorations) + }), + ] + }, +}) + +export const getStoredSelection = (state: EditorState) => + state.field(storedSelectionState) + +export const setStoredSelection = (selection: EditorSelection | null) => { + return { + effects: [ + storedSelectionEffect.of(selection), + // TODO: only disable selection highlighting if the current selection is a search match + highlightSelectionMatchesConf.reconfigure( + selection ? [] : highlightSelectionMatchesExtension + ), + ], + } +} + +const highlightSelectionMatchesConf = new Compartment() + +const highlightSelectionMatchesExtension = highlightSelectionMatches({ + wholeWords: true, +}) + +// store the search query for use when switching between files +// TODO: move this into EditorContext? let searchQuery: SearchQuery | null export const search = () => { @@ -48,14 +110,13 @@ export const search = () => { return [ // keymap for search - keymap.of( - searchKeymap.filter( - item => !item.key || !ignoredSearchKeybindings.has(item.key) - ) - ), + keymap.of(searchKeymap), // highlight text which matches the current selection - highlightSelectionMatches(), + highlightSelectionMatchesConf.of(highlightSelectionMatchesExtension), + + // a stored selection for use in "within selection" searches + storedSelectionState, // a wrapper round `search`, which creates a custom panel element and passes it to React by dispatching an event searchExtension({ @@ -285,6 +346,22 @@ export const search = () => { '.ol-cm-search-replace-buttons': { order: 4, }, + '.ol-cm-stored-selection': { + background: 'rgba(125, 125, 125, 0.1)', + paddingTop: 'var(--half-leading)', + paddingBottom: 'var(--half-leading)', + }, + // set the default "match" style + '.cm-selectionMatch, .cm-searchMatch': { + backgroundColor: 'transparent', + outlineOffset: '-1px', + paddingTop: 'var(--half-leading)', + paddingBottom: 'var(--half-leading)', + }, + // make sure selectionMatch inside searchMatch doesn't have a background colour + '.cm-searchMatch .cm-selectionMatch': { + backgroundColor: 'transparent !important', + }, }), ] } diff --git a/services/web/frontend/js/features/source-editor/extensions/theme.ts b/services/web/frontend/js/features/source-editor/extensions/theme.ts index 7ad8ad0f9e..2039aff297 100644 --- a/services/web/frontend/js/features/source-editor/extensions/theme.ts +++ b/services/web/frontend/js/features/source-editor/extensions/theme.ts @@ -195,20 +195,6 @@ const staticTheme = EditorView.theme({ marginLeft: '-1px', // half the border width borderLeftColor: 'inherit', }, - // set the default "selection match" style - '.cm-selectionMatch, .cm-searchMatch': { - backgroundColor: 'transparent', - outlineOffset: '-1px', - }, - // make sure selectionMatch inside searchMatch doesn't have a background colour - '.cm-searchMatch .cm-selectionMatch': { - backgroundColor: 'transparent !important', - }, - // Match the height of search matches to selection matches - '.cm-searchMatch': { - paddingTop: 'var(--half-leading)', - paddingBottom: 'var(--half-leading)', - }, // remove border from hover tooltips (e.g. cursor highlights) '.cm-tooltip-hover': { border: 'none', diff --git a/services/web/locales/en.json b/services/web/locales/en.json index 33d699f667..3eb84a8852 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -1298,6 +1298,7 @@ "search_replace_with": "Replace with", "search_search_for": "Search for", "search_whole_word": "Whole word", + "search_within_selection": "Within selection", "secondary_email_password_reset": "That email is registered as a secondary email. Please enter the primary email for your account.", "security": "Security", "see_changes_in_your_documents_live": "See changes in your documents, live", diff --git a/services/web/package.json b/services/web/package.json index 1c2ecd4ac7..64d8553e41 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -76,7 +76,7 @@ "@codemirror/lang-markdown": "^6.1.1", "@codemirror/language": "^6.6.0", "@codemirror/lint": "^6.2.1", - "@codemirror/search": "^6.4.0", + "@codemirror/search": "github:overleaf/codemirror-search#ea83364b22ad66455fc94babea7d576fa9f76a93", "@codemirror/state": "^6.2.0", "@codemirror/view": "^6.9.6", "@contentful/rich-text-html-renderer": "^16.0.2", diff --git a/services/web/test/frontend/features/source-editor/components/codemirror-editor.spec.tsx b/services/web/test/frontend/features/source-editor/components/codemirror-editor.spec.tsx index a47de12687..cb5ef71d51 100644 --- a/services/web/test/frontend/features/source-editor/components/codemirror-editor.spec.tsx +++ b/services/web/test/frontend/features/source-editor/components/codemirror-editor.spec.tsx @@ -441,6 +441,24 @@ describe('', { scrollBehavior: false }, function () { cy.get('@replace-input').clear() cy.should('not.contain.text', 'abcde') + // replace all within selection + cy.get('@search-input').clear().type('contentLine') + cy.get('.ol-cm-search-form-position').should('have.text', '1 of 100') + cy.get('.cm-line') + .eq(27) + .should('contain.text', 'contentLine 0') + .click() + .type('{shift}{downArrow}{downArrow}{downArrow}') + cy.findByLabelText('Within selection').click() + cy.get('.ol-cm-search-form-position').should('have.text', '1 of 3') + cy.get('@replace-input').clear().type('contentedLine') + cy.findByRole('button', { name: /replace all/i }).click() + cy.get('.cm-line:contains("contentedLine")').should('have.length', 3) + cy.findByLabelText('Within selection').click() + cy.get('.ol-cm-search-form-position').should('have.text', '2 of 97') + cy.get('@search-input').clear() + cy.get('@replace-input').clear() + // close the search form, to clear the stored query cy.findByRole('button', { name: 'Close' }).click() }) @@ -465,9 +483,11 @@ describe('', { scrollBehavior: false }, function () { cy.get('[type="checkbox"][name="caseSensitive"]').as('case-sensitive') cy.get('[type="checkbox"][name="regexp"]').as('regexp') cy.get('[type="checkbox"][name="wholeWord"]').as('whole-word') + cy.get('[type="checkbox"][name="withinSelection"]').as('within-selection') cy.get('label').contains('Aa').as('case-sensitive-label') cy.get('label').contains('[.*]').as('regexp-label') cy.get('label').contains('W').as('whole-word-label') + cy.findByLabelText('Within selection').as('within-selection-label') cy.findByRole('button', { name: 'Replace' }).as('replace') cy.findByRole('button', { name: 'Replace All' }).as('replace-all') cy.findByRole('button', { name: 'next' }).as('find-next') @@ -480,6 +500,7 @@ describe('', { scrollBehavior: false }, function () { cy.get('@case-sensitive').should('be.focused').tab() cy.get('@regexp').should('be.focused').tab() cy.get('@whole-word').should('be.focused').tab() + cy.get('@within-selection').should('be.focused').tab() cy.get('@find-next').should('be.focused').tab() cy.get('@find-previous').should('be.focused').tab() cy.get('@replace').should('be.focused').tab() @@ -491,6 +512,7 @@ describe('', { scrollBehavior: false }, function () { cy.get('@replace').should('be.focused').tab({ shift: true }) cy.get('@find-previous').should('be.focused').tab({ shift: true }) cy.get('@find-next').should('be.focused').tab({ shift: true }) + cy.get('@within-selection').should('be.focused').tab({ shift: true }) cy.get('@whole-word').should('be.focused').tab({ shift: true }) cy.get('@regexp').should('be.focused').tab({ shift: true }) cy.get('@case-sensitive').should('be.focused').tab({ shift: true }) @@ -501,6 +523,7 @@ describe('', { scrollBehavior: false }, function () { '@case-sensitive-label', '@regexp-label', '@whole-word-label', + '@within-selection-label', ]) { // Toggle when clicked, then focus the search input cy.get(option).click().should('have.class', 'checked')