-
-
Notifications
You must be signed in to change notification settings - Fork 777
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
Added Tools Filter to Projects Check Page #5444
Added Tools Filter to Projects Check Page #5444
Conversation
Want to review this pull request? Take a look at this documentation for a step by step guide! From your project repository, check out a new branch and test the changes.
|
Availability: Weekdays |
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.
@pdimaano , Tools filter is added and works as intended in tablet and desktop viewport. Nice work. Thank you !
This comment may be out of scope for this issue, but I notice that "AWS" appears twice in "Technologies" and once in "Tools". |
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.
Hey @pdimaano - Great work- you have covered the bases with the branches, linking the issue, the descriptions, and providing the screenshots. Bonus points for correcting the spelling mistakes in the comments!
But there is something odd happening with the filtering itself and the filter counts- for example, the filter shows "AWS (3)" in "Technologies and "AWS - " in "Tools". If you would, please track down the source of the problem.
Otherwise, this is looking good. Thanks for working on this issue
Just noticed your comment @Thinking-Panda |
@t-will-gillis Thank you for the feedback. I see that there is a discrepancy on some of the cards on the projects check page where AWS is in the "Technologies" category in some and the "Tools" category in others. I believe this is causing the multiple AWS options to be dynamically generated in the separate dropdown lists. Is this something that needs to be changed here or is it beyond the scope of this issue like what @Thinking-Panda stated previously? |
The discrepancy is based on how AWS is listed in each of the projects' markdown file in |
Hi @pdimaano With the current changes to the markdowns in the This is something that is in the scope of the issue because the filters are not functioning correctly yet. |
Hi @pdimaano Here is something to try: since we know that we will want all "AWS" in "Technologies" in the end, you can check how the filter performs if you move "AWS" to
If that appears to work, then I think it will be fine to add those changes to your PR and call it good... |
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.
Thanks, @pdimaano
* added code to dynamically render list for tools filter * updated js with tools filter and html to display proper cards * moved AWS to technologies section * moved AWS to technologies section * added technologies section before transferring AWS to new section
Fixes #4427
What changes did you make?
Why did you make the changes (we will use this info to test)?
Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)
Visuals before changes are applied
Visuals after changes are applied