-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Put items into sections #5348
base: main
Are you sure you want to change the base?
Put items into sections #5348
Conversation
merge main update into registry_page
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a really good start, thank you.
A few inline comments. Most of them are related to changes you have applied to files that shouldn't be there. You might be able to revert all of them by running npm run fix:format
locally.
A few more comments:
- I understand why you did it that way (we say "libraries, plugins, integrations"), but by applying a search on those items you get a selection of items that not always fits the description. This is OK for now, but we need to think about addressing that differently. I will think about that
- The links at the bottom of an item are off, can you take a look that they are in line with the rest of the element?
- If I make the screen size smaller the elements become very thin, I think it should switch to 1 item per row with an earlier width-selector
@svrnm I trust you're doing fine. Happy weekend. I have made the changes you requested. I noticed some of those cards weren't displaying the quick install code so I worked on it. Also, would you advise that all the cards being displayed have a "quick install" to ensure all components are aligned together? This is also a gentle reminder for the suggestion regarding the "search that returns items that do not always fit the description". |
Thank you @Beccamak! This looks really good, I consider this as done! The changes required for making this work with the current solution is out of scope for your work. When the application phase for outreachy is over, we can revisit this and talk about making this work, if you want to, but since we do not merge PRs until the end of the application phase, let's keep it as is. Note, that if you want to get the files, linter CI issues resolved run |
@svrnm Thank you so much. This is well noted and understood. I've learned a lot by just trying to implement some of those things. I'll move on to selecting an optional task now. |
While I still like the idea of this PR, I am yet unsure how to incorporate it. The categories "Libraries, Plugins, Integrations" do not work, since they do not match any of the categories we have as of today, but we could do "Instrumentation Libraries, "Collector Components", "Application Integrations" and a few other ones more fitting. This would also allow us to add some words and explaining them better. This will be some extra work, are you still interested in helping with that @Beccamak ? cc @open-telemetry/docs-approvers WDYT? |
@svrnm Yes, I'm still interested in helping out. |
A few thoughts on that:
|
@svrnm Looking at the first suggestion, we want to have a small top paragraph for Instrumentation Library, Collector Components and Utilities letting the end user know what they will get from them, what they are etc... right? So that would come before the search bar? |
Below the Search bar, it would look like this: SEARCHBAR Title 1 Description 1 Example of 1 | Example of 1 | Example of 1 (show more) Title 2 Description 2 Example of 2 | Example of 2 | Eample of 2 (show more) ... |
#5317
@svrnm I have created sections for the major categories on the registry page.
For view all links, I had to use the search functionality that was already implemented. Initially, I thought that on the backend, there was an identifier or tag for entries that easily identifies them as plugins, libraries, or instrumentations. The plan was to have all these categories filtered out and then on clicking "view all" they are populated.
However, after some interactions with the codebase, I didn't see that there. I noticed that if I had to get a category out (for example, the libraries), it had to be through a search (the fuzzy search the system is using), and I couldn't find a way of passing the filtered results back to the template, which I think explains why the whole entries were initially inserted on the registry page. The keys were compared to filter out the keys (entries) that didn't match the keys (entries) in the returned search results on the DOM.
Kindly let me know your thoughts on this.