opentelemetry.io icon indicating copy to clipboard operation
opentelemetry.io copied to clipboard

[outreachy] Registry: click on label for additional filtering

Open basiratkareem opened this issue 1 year ago • 6 comments

Task This update allows users to filter records by clicking on a tag (like native, sandbox, etc.), enabling quicker access to filtered records without needing to use the search bar. closes #5312

Implementation

  • Replaced <span> tags with <button> elements in layouts/partials/ecosystem/registry/entry.html
  • Applied existing Bootstrap styles for consistency with the rest of the project.
  • Added a function applyTagFilter in assets/js/registrySearch.js to handle the filter action when a tag is clicked.
  • Added a title to the buttons(tags). On hover, each button displays "Click to filter by [tag name]".

Result The tags native, first party integration, sandbox, graduated, and deprecated are now clickable buttons.
Clicking a tag immediately filters the records in the OpenTelemetry registry based on the selected tag, enhancing the filtering experience.

basiratkareem avatar Oct 08 '24 00:10 basiratkareem

CLA Signed

The committers listed above are authorized under a signed CLA.

  • :white_check_mark: login: basiratkareem / name: Basirat Kareem (88f811b62605a20ad6386be879a50c50002aed04, 3d10a65ab059178cfc8eda141ecf5b7320527b55, fecfd0e897faa9f348876d3a304c10b9e70466ee, 4a3cacc479033c69532f35c883f25026cc06fd64, 31bb474fdbbea76fc18d3ca7be25a15082fffdb3, 1f0de0483303024e07bab2a9630a02a869cec7b0, aa84339d29521bbba5b6dbb39c871e9a789174f9, 46a642477f2e325ec481f164a18858bccced1435, 680eb59d74d7a9ae0b5ac7201915d364047e6406, 3a3b4940d7e5086c4f31edf37fee6a6b3ecb5000, b80101d5144b112e20468295b03c29ac28410c07, 52386d1619e0eb40c6f3a1b8226ed9958f57d2b3)

@basiratkareem checking in if you could make some progress here? Today is the last day I can help you with moving this PR forward, so let me know if there is anything I can do to help you

svrnm avatar Oct 25 '24 06:10 svrnm

@basiratkareem checking in if you could make some progress here? Today is the last day I can help you with moving this PR forward, so let me know if there is anything I can do to help you

Hi @svrnm,

Thank you for your continued support and feedback! I aplogiize for the delay response. I’ve made several adjustments to the code, including tweaks to the event handling. Unfortunately, these updates haven’t resolved the issue, and the functionality is still the same as what we both observed in the Netlify preview.

Initially, running npm run serve:netlify gave the expected output on my end. However, after restarting my device and retesting, it’s now producing the same result as npm run serve.

I suspect the issue might be due to the URL updating after the initial click. If there’s anything specific you’d suggest trying or if there’s something I might be missing, I’d be glad to explore it to resolve this within your timeline.

Thank you again for your guidance and patience.

basiratkareem avatar Oct 25 '24 08:10 basiratkareem

the problem is the following:

  • we show a "#default-body" div if no search has been applied, your code adds the event listeners to the items in that list
  • we show a "#search results" div if search has been applied, they are lazy loaded, so your code is not applied to them.

You might need to update your code in a way that it re-applies the addition of event listeners to the newly created elements after the search was executed

svrnm avatar Oct 25 '24 09:10 svrnm

the problem is the following:

  • we show a "#default-body" div if no search has been applied, your code adds the event listeners to the items in that list
  • we show a "#search results" div if search has been applied, they are lazy loaded, so your code is not applied to them.

You might need to update your code in a way that it re-applies the addition of event listeners to the newly created elements after the search was executed

I will check this out.

basiratkareem avatar Oct 25 '24 11:10 basiratkareem

Hello @svrnm, I have added applyTag function to the code and updated the executeSearch function to call applyTag after populating search results. I have tested the behaviour and it is working as expected. Clicking the labels initiates a search. I look forward to your feedback.

basiratkareem avatar Oct 25 '24 18:10 basiratkareem

hey @basiratkareem,

I would like to take that PR into the main branch and with that into the project. But we need to rework it a little bit, would you still be interested in that?

Here's what needs to be changed changed: I merged https://github.com/open-telemetry/opentelemetry.io/pull/5328, which introduces a "flag" parameter, so far only for "deprecated", "native" and "first party", not the different CNCF projects, can you make use of that instead of appending it to the search term? We can skip the CNCF projects to simplify it for now.

Thanks.

svrnm avatar Nov 11 '24 15:11 svrnm

hey @basiratkareem,

I would like to take that PR into the main branch and with that into the project. But we need to rework it a little bit, would you still be interested in that?

Here's what needs to be changed changed: I merged #5328, which introduces a "flag" parameter, so far only for "deprecated", "native" and "first party", not the different CNCF projects, can you make use of that instead of appending it to the search term? We can skip the CNCF projects to simplify it for now.

Thanks.

Hello @svrnm I'm happy to continue working on this. Based on my understanding of your comment:

  1. A similar PR #5328 has been merged, which implemented a flag parameter for the filter.
  2. In PR #5328, I noticed the filter uses a dropdown, similar to the component and types.
  3. My task will be to integrate the flag parameter, ensuring the filter is initiated when the flag button is click without adding the flag value to the search input.

please let me know if I am missing something.

I'll make the necessary changes on my end and provide an update on this. Thank you.

basiratkareem avatar Nov 12 '24 13:11 basiratkareem

1. A similar PR [add a "flag" filter to registry  #5328](https://github.com/open-telemetry/opentelemetry.io/pull/5328) has been merged, which implemented a flag parameter for the filter.

2. In PR [add a "flag" filter to registry  #5328](https://github.com/open-telemetry/opentelemetry.io/pull/5328), I noticed the filter uses a dropdown, similar to the component and types.

3. My task will be to integrate the flag parameter, ensuring the filter is initiated when the flag button is click without adding the flag value to the search input.

please let me know if I am missing something.

I'll make the necessary changes on my end and provide an update on this. Thank you.

Your understanding is correct 👍

svrnm avatar Nov 12 '24 14:11 svrnm

Hi @svrnm

I’ve integrated the flag parameter into the code, and the filter is now activated when the flag button is clicked. The page is populated accordingly, and the search input field remains unchanged. I’ve also recorded a screen recording to demonstrate the behaviour.

https://www.loom.com/share/3427b2b7965a479c800175b63048d6ba?sid=8db7958f-6ca2-41f5-aa02-2d44d55ac397

Thank you, and please let me know your thoughts!

basiratkareem avatar Nov 14 '24 12:11 basiratkareem

I took a look at your code, but from what I see you didn't update your branch to be in sync with main, so the changes introduced via #5328 are not part of your work here, it also creates a merge conflict.

Please do the following:

  • backup your work here
  • bring your fork at https://github.com/basiratkareem/opentelemetry.io/ up-to-date with upstream (https://github.com/open-telemetry/opentelemetry.io)
  • create a new local clone and a new local branch and incorporate your work accordingly.

https://www.loom.com/share/3427b2b7965a479c800175b63048d6ba?sid=8db7958f-6ca2-41f5-aa02-2d44d55ac397

Thanks for taking the time to take a video, but not that we automatically create a preview of your work, which is accessible at https://deploy-preview-5340--opentelemetry.netlify.app/ecosystem/registry/, so approvers can try out your work directly.

svrnm avatar Nov 14 '24 14:11 svrnm

I took a look at your code, but from what I see you didn't update your branch to be in sync with main, so the changes introduced via #5328 are not part of your work here, it also creates a merge conflict.

Please do the following:

  • backup your work here
  • bring your fork at https://github.com/basiratkareem/opentelemetry.io/ up-to-date with upstream (https://github.com/open-telemetry/opentelemetry.io)
  • create a new local clone and a new local branch and incorporate your work accordingly.

https://www.loom.com/share/3427b2b7965a479c800175b63048d6ba?sid=8db7958f-6ca2-41f5-aa02-2d44d55ac397

Thanks for taking the time to take a video, but not that we automatically create a preview of your work, which is accessible at https://deploy-preview-5340--opentelemetry.netlify.app/ecosystem/registry/, so approvers can try out your work directly.

Apologies for that. I will follow the steps, make the required updates, and revert as needed. Thank you for your assistance!

basiratkareem avatar Nov 14 '24 14:11 basiratkareem

Hi @svrnm, changes implemented in PR #5611

basiratkareem avatar Nov 15 '24 09:11 basiratkareem