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

Some minor improvements for Find in page #959

Merged
merged 2 commits into from
Sep 6, 2023
Merged

Some minor improvements for Find in page #959

merged 2 commits into from
Sep 6, 2023

Conversation

HollowMan6
Copy link
Collaborator

  • Stop having findInPage and Panel opened at same time

    We shouldn't let user feel that we can search content in the library page through findInPage. So we close findInPage when we open the Library, and we close the panel when we open findInPage in the hamburger menu.

    We shall pay attention to this as well in Reorganize Libraries and add search UI in panels #881 once this is merged

  • Auto focus when findInPage opened; Clear text when hidden

We may want to keep findInPage opened even if the page changes (such as reloading/navigating to new page), to keep consistency with other browsers like Chrome.

We shouldn't let user feel that we can search content
in the library page through findInPage. So we close
findInPage when we open the Library, and we close the
panel when we open findInPage in the hamburger menu.

We shall pay attention to this as well in
#881
once this is merged

Signed-off-by: Songlin Jiang <[email protected]>
Copy link
Member

@javifernandez javifernandez left a comment

Choose a reason for hiding this comment

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

Tested it in Oculus and it work as expected.

@svillar svillar merged commit 7a6931f into main Sep 6, 2023
7 checks passed
@svillar svillar deleted the findInPage branch September 6, 2023 10:12
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

Successfully merging this pull request may close these issues.

3 participants