Skip to content

fix: Improve search filter sorting, searching, and add exclude #698

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.

Sign up for GitHub

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

Jump to bottom
Merged
kodiakhq merged 8 commits into from
Mar 24, 2025
Merged

fix: Improve search filter sorting, searching, and add exclude #698

kodiakhq merged 8 commits into from
Mar 24, 2025

Conversation

@teeohhem
Copy link
Contributor

@teeohhem teeohhem commented Mar 20, 2025 •
edited
Loading

Adds

  • NOT button next to the "Only" button that appears on hover of a search filter item. This will exclude the item from the search
  • Search Filter filtering - restores filtering the search filter options by the text entered by the user

Enhances:

  • Search filter ordering - selected items will appear on top, followed by an alphabetically ordered list of search filter items

New NOT option
Before
image

After
image
image

Search filter ordering
Before
image

After
image

Searching within Filters
image
image

@changeset-bot
Copy link

changeset-bot bot commented Mar 20, 2025 •
edited
Loading

⚠️ No Changeset found

Latest commit: 3fecd29

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@teeohhem teeohhem marked this pull request as ready for review March 20, 2025 17:33
ernestii
ernestii approved these changes Mar 22, 2025
View reviewed changes
packages/app/src/components/DBSearchPageFilters.tsx
Tooltip,
UnstyledButton,
} from '@mantine/core';
import { IconSearch } from '@tabler/icons-react';
Copy link
Contributor

@ernestii ernestii Mar 22, 2025

Choose a reason for hiding this comment

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

heck yeah

packages/app/src/components/DBSearchPageFilters.tsx
// taken care by the onClick in the group, triggering here will double fire
emptyFn
}
indeterminate={value === 'excluded'}
Copy link
Contributor

@ernestii ernestii Mar 22, 2025

Choose a reason for hiding this comment

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

nit: maybe also change the color of the checkbox when it's excluded?

packages/app/src/components/__tests__/DBSearchPageFilters.tsx
};

it('should sort options alphabetically by default', () => {
renderWithMantine(<FilterGroup {...defaultProps} />);
Copy link
Contributor

@ernestii ernestii Mar 22, 2025

Choose a reason for hiding this comment

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

👀 noice

packages/app/styles/SearchPage.module.scss
flex-shrink: 0;
word-break: break-all;

:global {
Copy link
Contributor

@ernestii ernestii Mar 22, 2025

Choose a reason for hiding this comment

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

nit: this is probably better to move to a global 'mantine-overrides.scss' file? otherwise there might be wrong styling when SearchPage isn't imported

@kodiakhq kodiakhq bot merged commit 02ee0a3 into v2 Mar 24, 2025
4 checks passed
@kodiakhq kodiakhq bot deleted the tom/filter-improvements branch March 24, 2025 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@ernestii ernestii ernestii approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

automerge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@teeohhem @ernestii @MikeShi42