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.

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

The fix for issue #78 is not complete #98

Closed
xybei opened this issue Jun 24, 2024 · 6 comments
Closed

The fix for issue #78 is not complete #98

xybei opened this issue Jun 24, 2024 · 6 comments

Comments

@xybei
Copy link
Contributor

xybei commented Jun 24, 2024

The fix for issue #78 does not seem to be very friendly to Chinese characters.
When I input and search for Chinese, I need to type several letters "zhong" to enter the Chinese character "中" (for example, it appears on the first page), but when I type the first letter "z" (it appears on the fifth page), the input event will be triggered and the search will be started, the view will jump to the fifth page. Even if I enter the complete character "中" later, the character "中" on the first page has been skipped.

@jamie-lemon
Copy link
Collaborator

@xybei Sorry for the late reply - are you able to provide a sample document that we can test against?

@xybei
Copy link
Contributor Author

xybei commented Aug 28, 2024

@jamie-lemon Here is a sample file: test-search.pdf

This is a GIF I recorded to reproduce the problem. If you want to reproduce the problem, you need to use a Chinese input method (such as the Microsoft Pinyin input method in Windows).
chrome-capture-2024-8-28-3

This problem is caused by changing the onchange event to oninput when fixing issue #78. We should keep using onchange, that is, wait until the input is completed before starting the search. I think issue #78 should be solved by optimizing the processing logic of current_search_page.

jamie-lemon added a commit that referenced this issue Aug 28, 2024
Addresses issues #78 & #98
A listener for search input doesn't work well
with language input methods which convert
words into kanji or Chinese characters.
This adds a dedicated search button and
ensures that the previous & next buttons
are shown and hidden as required.
It also prints out better syntax for the search
information.
@jamie-lemon
Copy link
Collaborator

@xybei As this is a complicated UI issue I think the best solution is to not listen for "on input" or "on change" events on the search text field as this will always be problematic.
Please check out the branch referenced on #111 and let me know how it works for you.

xybei added a commit to xybei/mupdf.js that referenced this issue Aug 30, 2024
* Addresses issues ArtifexSoftware#78 & ArtifexSoftware#98
  Fix the issue where the search results on the current page would be skipped when clicking the "Next" search button for the first time.
  Improve the search experience for the Chinese input method.

* Add an Enter key handler for the search input.
  Enter: search "Next"
  Shift + Enter: search "Prev"

* Display page number based on 1
xybei added a commit to xybei/mupdf.js that referenced this issue Aug 30, 2024
* Addresses issues ArtifexSoftware#78 & ArtifexSoftware#98
  Fix the issue where the search results on the current page would be skipped when clicking the "Next" search button for the first time.
  Improve the search experience for the Chinese input method.

* Add an Enter key handler for the search input.
  Enter: search "Next"
  Shift + Enter: search "Prev"

* Display page number based on 1

Signed-off-by: xybei <[email protected]>
@xybei
Copy link
Contributor Author

xybei commented Aug 30, 2024

Thank you @jamie-lemon. Adding a "Search" button is an innovative idea, but there is a small flaw in my test.

If the target word only appears in the pages before the current page, clicking "Search" cannot trigger the display of "Prev" or "Next", and thus cannot search for the previous words.

Considering that search boxes usually use two buttons, I submitted a pull request #112. Please see if it is acceptable?

@jamie-lemon
Copy link
Collaborator

Sure - I will check it out and get back to you. 👍

jamie-lemon pushed a commit that referenced this issue Sep 3, 2024
* Improve search functionality

* Addresses issues #78 & #98
  Fix the issue where the search results on the current page would be skipped when clicking the "Next" search button for the first time.
  Improve the search experience for the Chinese input method.

* Add an Enter key handler for the search input.
  Enter: search "Next"
  Shift + Enter: search "Prev"

* Display page number based on 1

Signed-off-by: xybei <[email protected]>

* Replace the Prev & Next button text with the language agnostic symbols '<' and '>'

Signed-off-by: xybei <[email protected]>

---------

Signed-off-by: xybei <[email protected]>
@xybei
Copy link
Contributor Author

xybei commented Sep 3, 2024

This issue has been addressed by pull request #112

@xybei xybei closed this as completed Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants