Skip to content

feat(web): focus on face-editor search input#27136

Open
cratoo wants to merge 4 commits intoimmich-app:mainfrom
cratoo:face-search-focus
Open

feat(web): focus on face-editor search input#27136
cratoo wants to merge 4 commits intoimmich-app:mainfrom
cratoo:face-search-focus

Conversation

@cratoo
Copy link
Contributor

@cratoo cratoo commented Mar 21, 2026

Description

To further optimize #26826 the face-search input now is focused when opened. Also ensured to keep the focus on the search if the face-box is moved or resized (which most likely will always happen). Escape-key handling is also kept, so one can still close the face-editor.

How Has This Been Tested?

  • Open face-editor with shortcut 'p' or by clicking on the +-button.
  • Check if focus is on the search input.
  • Move or resize face-editor and check if focus stays/returns to search input.
  • Press escape and check that face-editor closes and returns to photo-viewer.

Screenshots (if appropriate)

Checklist:

  • I have carefully read CONTRIBUTING.md
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if applicable
  • I have no unrelated changes in the PR.
  • I have confirmed that any new dependencies are strictly necessary.
  • I have written tests for new code (if applicable)
  • I have followed naming conventions/patterns in the surrounding code
  • All code in src/services/ uses repositories implementations for database calls, filesystem operations, etc.
  • All code in src/repositories/ is pretty basic/simple and does not have any immich specific logic (that belongs in src/services/)

Please describe to which degree, if any, an LLM was used in creating this pull request.

AI was used to check codebase, if there are other ways to do focus-related things. As per my understanding the approach with svelte-tick is used in similar cases.

};

const focusSearchInput = () => {
searchContainerEl?.querySelector('input')?.focus();
Copy link
Member

Choose a reason for hiding this comment

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

Can't you bind: to the Input directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah sure, you're right. But shall I only move it to the Input or also get rid of the focusSearchInput() altogether and call it inline in the mount and the effect?
As said in the description at the first glance it felt more consistent to the other parts, but maybe I was mislead there are too many superfluous parts in there now.

Copy link
Member

Choose a reason for hiding this comment

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

I would probably just inline it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bind now on input and inlined it. Thanks for your feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants