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

feat: search by shortcut #2594

Merged
merged 1 commit into from
Mar 7, 2024
Merged

feat: search by shortcut #2594

merged 1 commit into from
Mar 7, 2024

Conversation

mstykow
Copy link
Member

@mstykow mstykow commented Mar 6, 2024

Summary of changes

Context and reason for change

closes #2587

How can the changes be tested

Press ctrl/cmd+f in all four panels where searching is possible.

@@ -14,7 +14,7 @@ export const TABS_CONTAINER_HEIGHT = 30;

export const Panel = styled(MuiBox)({
flex: 1,
overflowY: 'auto',
overflowY: 'hidden',
Copy link
Member Author

Choose a reason for hiding this comment

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

this sometimes displayed an unnecessary scroll bar for a split second while the attributions/signals panel was updating.

[activeRelation, attributionIds, attributions, multiSelectedAttributionIds],
);
const areAllAttributionsSelected = useMemo(() => {
const activeAttributionIds = attributionIds?.filter(
Copy link
Member Author

Choose a reason for hiding this comment

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

we need to filter by active relation first in order to prevent showing the select-all checkbox for a split second while the attributions/signals panel is updating and the active relation may be changing.

@mstykow mstykow force-pushed the feat-search-by-shortcut branch from 39a611d to d2a8a4e Compare March 7, 2024 08:21
paddingRight: value ? '26px' : '0px',
paddingLeft: '26px',
paddingRight: value ? '24px' : '0px',
paddingLeft: '24px',
Copy link
Member Author

Choose a reason for hiding this comment

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

this makes sure the search button is a square

@vasily-pozdnyakov
Copy link
Contributor

As discussed, I find the current implementation not really useful as I have to set focus on specific panel first. Instead, I would directly click on the search icon.
My proposal:

  1. ctrl+f is actionable in any place and always navigates to the resource search.
  2. hitting tab brings user directly to the next search (resources->linked resources->attributions->signals)

@mstykow mstykow force-pushed the feat-search-by-shortcut branch from d2a8a4e to 92f68af Compare March 7, 2024 13:19
@mstykow
Copy link
Member Author

mstykow commented Mar 7, 2024

As discussed, I find the current implementation not really useful as I have to set focus on specific panel first. Instead, I would directly click on the search icon. My proposal:

  1. ctrl+f is actionable in any place and always navigates to the resource search.
  2. hitting tab brings user directly to the next search (resources->linked resources->attributions->signals)

I've added four different global hotkeys now, one for each search.

@mstykow mstykow force-pushed the feat-search-by-shortcut branch from 92f68af to d74b0f0 Compare March 7, 2024 13:50
Copy link
Contributor

@vasily-pozdnyakov vasily-pozdnyakov left a comment

Choose a reason for hiding this comment

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

looks good. See failing tests and the small comment from my side.

- add four global hotkeys for accessing the four different searches
- introduce ctrl/cmd+f shortcut to search for resources, attributions, signals depending on the focused context
- improve general keyboard accessibility
- use React context for search-ref because Redux can only handle serializable data
- use context for Virtuoso comp. props as they cannot be inlined: petyosi/react-virtuoso#566

closes #2587

Signed-off-by: Maxim Stykow <maxim.stykow@tngtech.com>
@mstykow mstykow force-pushed the feat-search-by-shortcut branch from d74b0f0 to 4daded9 Compare March 7, 2024 16:37
@mstykow mstykow merged commit 06ea18a into main Mar 7, 2024
5 checks passed
@mstykow mstykow deleted the feat-search-by-shortcut branch March 7, 2024 17:13
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.

Reintroduce "cmd+f" shortcut for quick search of resources
2 participants