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

Change dataset filters to Patternfly Attribute Filter pattern: Fixes: #1346 #1797

Merged
merged 1 commit into from
Jun 23, 2024

Conversation

johnaohara
Copy link
Member

This PR changes the filter dropdown to use the Patternfly attribute-search pattern

Fixes Issue

Fixes: #1346
Screenshot from 2024-06-16 13-52-43

Screenshot from 2024-06-16 13-52-46

@johnaohara johnaohara self-assigned this Jun 16, 2024
@johnaohara johnaohara changed the title Change dataset filters to Patternly Attribute Filter pattern Change dataset filters to Patternfly Attribute Filter pattern: Fixes: #1346 Jun 16, 2024
@johnaohara johnaohara marked this pull request as ready for review June 21, 2024 11:01
@johnaohara johnaohara requested a review from lampajr June 21, 2024 11:02
Copy link
Member

@lampajr lampajr left a comment

Choose a reason for hiding this comment

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

Hey @johnaohara, this looks an awesome improvement in the UI ❤️

I left a couple of comments.

horreum-web/src/components/CustomTable.tsx Outdated Show resolved Hide resolved
horreum-web/src/domain/runs/TestDatasets.tsx Outdated Show resolved Hide resolved
Comment on lines +87 to +90
props.options.map((option) => (
<MenuItem isSelected={props.selection === option} itemId={option} key={option}>
{option}
</MenuItem>
Copy link
Member

Choose a reason for hiding this comment

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

We need to filter out undefined options otherwise we would have null options (that are selected by default):
image

Suggested change
props.options.map((option) => (
<MenuItem isSelected={props.selection === option} itemId={option} key={option}>
{option}
</MenuItem>
props.options.filter(option => option).map((option) => (
<MenuItem isSelected={props.selection === option} itemId={option} key={option}>
{option}
</MenuItem>

Copy link
Member Author

@johnaohara johnaohara Jun 22, 2024

Choose a reason for hiding this comment

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

The filter does remove undefined, but the api (maybe it is the json marshalling, idk atm) is returning a mixture of null and undefined, and it is the null values that are causing the issue.

Other LabelValues only return undefined for an unknown value.

My intention was to merge this PR as it is, and then fix the api problem that is returning the wrong data type for an unknown label value

Copy link
Member

Choose a reason for hiding this comment

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

Got it 👍

Copy link
Member

@lampajr lampajr left a comment

Choose a reason for hiding this comment

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

LGTM

@johnaohara johnaohara merged commit daf99ea into Hyperfoil:master Jun 23, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dataset filtering should use the patternfly design recommendation
2 participants