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

Implement clickable flag buttons to filter records without updating search term #5611

Merged
merged 9 commits into from
Nov 22, 2024

Conversation

basiratkareem
Copy link
Contributor

In this update, when a flag is clicked, the filter is applied immediately to update the displayed records based on selected flag value.

The flag value is not added to search input field and the search input field is cleared upon flag selection.

@basiratkareem basiratkareem requested a review from a team as a code owner November 15, 2024 09:30
@basiratkareem
Copy link
Contributor Author

Hi @svrnm I have made the changes as discussed in this PR. I await your feedback. Thank you.

Copy link
Member

@svrnm svrnm left a comment

Choose a reason for hiding this comment

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

a few inline comments.

Your code does not work as expected, when I click on "native" what I see are not all the items with the "native" flag set, but it still searches for "native" in the text of the element.

There is now an easy solution for you and one that is more complicated. The easy one is that you follow the example of the recently introduced tags: instead of having buttons you add a link that just sets the flag, e.g. <a href="/ecosystem/registry?flag={{ flagName }}" class="badge rounded-pill text-bg-danger text-white" title="..." ...>...

The more complicated solution would require you to update the ?flag= in the URL and then applies those changes. What I mean is that clicking that "native" or "deprecated" on an item should behave as if I click on "Flags > Deprecated" or "Flags > Native" in the search bar above, i.e. the elements in the red circles in the screenshot below should behave the same:

Screenshot 2024-11-18 at 08 39 05

layouts/partials/ecosystem/registry/entry.html Outdated Show resolved Hide resolved
layouts/partials/ecosystem/registry/entry.html Outdated Show resolved Hide resolved
Copy link
Member

@svrnm svrnm left a comment

Choose a reason for hiding this comment

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

Can you remove the empty lines in registrySearch.js? Otherwise it looks good to me.

assets/js/registrySearch.js Outdated Show resolved Hide resolved
assets/js/registrySearch.js Outdated Show resolved Hide resolved
assets/js/registrySearch.js Outdated Show resolved Hide resolved
assets/js/registrySearch.js Outdated Show resolved Hide resolved
@basiratkareem
Copy link
Contributor Author

Can you remove the empty lines in registrySearch.js? Otherwise it looks good to me.

Suggested changes have been committed. I am glad to hear that it looks good. Thank you

Copy link
Member

@svrnm svrnm left a comment

Choose a reason for hiding this comment

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

ah, one more thing!

layouts/partials/ecosystem/registry/entry.html Outdated Show resolved Hide resolved
layouts/partials/ecosystem/registry/entry.html Outdated Show resolved Hide resolved
layouts/partials/ecosystem/registry/entry.html Outdated Show resolved Hide resolved
@svrnm svrnm added this pull request to the merge queue Nov 22, 2024
Merged via the queue into open-telemetry:main with commit 65b7a32 Nov 22, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants