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.

Sign up for GitHub

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

Windows/macOS: Resolves #7520: Search field doesn't get focus when pressing Ctrl+F #7529

Merged
merged 2 commits into from
Dec 30, 2022
Merged

Windows/macOS: Resolves #7520: Search field doesn't get focus when pressing Ctrl+F #7529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0ae6214
fix: add useEffect to auto focus search bar when it is opened
betty-alagwu Dec 24, 2022
f2caa41
fix: focus when search bar is already open
betty-alagwu Dec 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/app-desktop/gui/NoteEditor/NoteEditor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ function NoteEditor(props: NoteEditorProps) {
showLocalSearch,
setShowLocalSearch,
searchMarkers: localSearchMarkerOptions,
} = useNoteSearchBar();
} = useNoteSearchBar({ noteSearchBarRef });

// If the note has been modified in another editor, wait for it to be saved
// before loading it in this editor.
Expand Down
7 changes: 5 additions & 2 deletions packages/app-desktop/gui/NoteEditor/commands/showLocalSearch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,11 @@ export const runtime = (comp: any): CommandRuntime => {
if (comp.editorRef.current && comp.editorRef.current.supportsCommand('search')) {
comp.editorRef.current.execCommand({ name: 'search' });
} else {
comp.setShowLocalSearch(true);
if (comp.noteSearchBarRef.current) comp.noteSearchBarRef.current.wrappedInstance.focus();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that you made focus() public, does it work if you just replace this with comp.noteSearchBarRef.current.focus()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@laurent22 no it doesn't. We can't focus immediately after calling comp.setShowLocalSearch(true) because React needs some time to rerender, so I used the useEffect instead and called noteSearchBarRef.current.focus() in the useEffect.

if (comp.noteSearchBarRef.current) {
comp.noteSearchBarRef.current.focus();
} else {
comp.setShowLocalSearch(true);
}
}
},
enabledCondition: 'oneNoteSelected',
Expand Down
15 changes: 13 additions & 2 deletions packages/app-desktop/gui/NoteEditor/utils/useNoteSearchBar.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useState, useCallback } from 'react';
import { useState, useCallback, MutableRefObject, useEffect } from 'react';
import Logger from '@joplin/lib/Logger';
import { SearchMarkers } from './useSearchMarkers';
const CommandService = require('@joplin/lib/services/CommandService').default;
Expand All @@ -25,10 +25,21 @@ function defaultLocalSearch(): LocalSearch {
};
}

export default function useNoteSearchBar() {
export interface UseNoteSearchBarProps {
noteSearchBarRef: MutableRefObject<any>;
}

export default function useNoteSearchBar({ noteSearchBarRef }: UseNoteSearchBarProps) {
const [showLocalSearch, setShowLocalSearch] = useState(false);
const [localSearch, setLocalSearch] = useState<LocalSearch>(defaultLocalSearch());


useEffect(() => {
if (showLocalSearch && noteSearchBarRef.current) {
noteSearchBarRef.current.focus();
}
}, [showLocalSearch, noteSearchBarRef]);

const onChange = useCallback((query: string) => {
// A query that's too long would make CodeMirror throw an exception
// which would crash the app.
Expand Down
1 change: 1 addition & 0 deletions packages/app-desktop/gui/NoteSearchBar.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class NoteSearchBar extends React.Component {
this.previousButton_click = this.previousButton_click.bind(this);
this.nextButton_click = this.nextButton_click.bind(this);
this.closeButton_click = this.closeButton_click.bind(this);
this.focus = this.focus.bind(this);

this.backgroundColor = undefined;
}
Expand Down