Make document search available in entity preview #3879
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Document search has always been a little difficult to discover, as it's hidden in entity previews. Additionally, when coming from a global search, searching for the same search query inside of a specific document requires more interactions than necessary (first need to expand the entity, then copy/paste the search query into the document search input. As a first step, this commit moves the search bar from the `PdfViewer` component up into the tab list in the `EntityViews` component. This makes it always available, no matter whether the entity is displayed in preview mode or not. This also saves some vertical space.
I can’t say that I fully understood why these styles where applied in the first place, so I'm not entirely sure this isn't breaking anything, but at first look everything seems okay.
tillprochaska
force-pushed
the
feature/2889-document-search
branch
from
783be1e
to
1f6f766
Compare
The `render` method for this component was huge. I’ve decided to split it up which should make it easier to understand what’s going on and to make changes.
This removes unnecessary indentation levels and just makes the code a little easier to read.
The previous solution was manually rendering just the markup for a tab panel outside of the `Tabs` component (because we don’t want a visible tab button in addition to the search input). This solution is a little more resilient, as it doesn’t rely on manually rendering HTML markup but uses the `Tab` component instead. Also, it provides a label for the search tab that can be used by assistive technology.
tillprochaska
force-pushed
the
feature/2889-document-search
branch
from
1f6f766
to
aadd33e
Compare
tillprochaska
commented
Oct 21, 2024
tillprochaska
commented
Oct 21, 2024
tillprochaska
commented
Oct 21, 2024
Comment on lines
+95
to
+99
| className={c( | ||
| Classes.ICON, | ||
| `${Classes.ICON}-document`, | ||
| 'PdfViewerSearch__icon' | ||
| )} |
There was a problem hiding this comment.
Why doesn’t this use the Icon component?
There was a problem hiding this comment.
For future reference:
The document icon from Blueprint looks like this: a single page. That’s what we want in this case.
However, we also have a custom set of icons, e.g. for the different FtM schemata which can be used with the Icon component in addition to the default Blueprint icons. As there is an FtM schema called Document, when using <Icon icon="document" /> you get this custom icon which is a multi-page document:
Because both the custom schema icon and the Blueprint icon are named document, this seems to be a workaround to get the default single-page icon from Blueprint instead of the custom multi-page icon.
tillprochaska
commented
Oct 21, 2024
tillprochaska
commented
Oct 21, 2024
This should be unnecessary as the reference modes aren't rendered in the first place in the top-level `render` method if `references` hasn't been loaded yet.
catileptic
approved these changes
Oct 28, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We frequently receive feedback about the search flow requiring too many clicks in the following use case:
This PR simplifies this flow as the document search is now available from within the entity previews. That means users can click on a search results, search within the document inside of the preview, and can easily return to the search results.
In a follow-up PR, I plan to prefill the search input for the document search with the value of the previous search or provide another easy way to reuse the search query so users don’t have to retype the entire query.
Screen.Recording.2024-10-21.at.16.58.57.mp4
From an implementation perspective, the main changes are in 52c706c. In addition to these changes, I’ve used the opportunity to refactor the
EntityViewscomponent as it was quite big and slightly confusing.For this reason, I’d recommend reviewing the commits individually. The commit messages also provide additional details.