Refactor no results message component 4903

[bphan002]

Fixes #4903

What changes did you make?

• The initial action was to extract the HTML code from toolkit.js, but that was already done and moved to current-guides.html. No style was needed.
• A noResultsMessageComponent function was created and placed under /assets/js/elements/noResultsMessageFilter.js as a reusable function for any pages that have a left-column filter. The first parameter takes the filterParams, and the second is the text color of the results since the various pages have different background colors.

Why did you make the changes (we will use this info to test)?

• A no-result message message does not display currently on the home or project page if the filter finds no results.
• Changes to the current message displayed used switch statements and were manually set depending on filterParams arguments passed. I temporarily removed this since I do not think this is a good way of doing it if we want a reusable function. Currently, the display is just the parameters argument passed in, which isn't ideal.
• The home page and project page use the same _includes file. Since this is the case I only showed the visual for the Project Page, but the functionality is the same for the Home Page.

Visuals before changes are applied to Projects Page

Visuals before changes are applied to Toolkit Page

Visuals after changes are applied to Projects Page(Notice the title before colon are dependent on the parameters passed into the function)

Visuals after changes are applied to toolkit Page(Notice the title before colon are dependent on the parameters passed into the function)

Revision may be needed if we want the parameters to be named differently. I'm opened to suggestions on how to tackle this.

NOTE TO REVIEWERS: The automated "Lint SCSS / Lint SCSS (pull_request)" check below should be ignored for your review. The linter itself is causing the scss error. - Will G.

[github-actions]

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.

git checkout -b bphan002-refactor-no-results-message-component-4903 gh-pages
git pull https://github.com/bphan002/hackforLAforkwebsite.git refactor-no-results-message-component-4903

[kwangric]

ETA: 9/6
Availability: All day 9/4 through 9/6

[Adastros]

Availability (PST): 10 - 11 PM 07SEP2023, 2 - 3 PM 09SEP2023, 10 - 11 PM 11-12SEP2023
Review ETA (PST): End of day 12SEP2023

[kwangric]

Hey @bphan002, thanks for taking on this extra large issue! I've had some time to briefly look at your pull request, but my week has been busier than anticipated and I probably won't finish a full review until this week.

One of the first things I noticed was that the search function on the Projects and Home page don't seem to be working anymore. Performing the search does add the parameter to the URL, but doesn't actually change the results of the projects or add the tags. Additionally, after attempting a search, the filters don't work correctly until the page is refreshed.

I'll take a closer look at the code to see what is causing this later this weekend, just wanted to let you know my findings in case you have a better idea of the cause.

[Adastros]

Hi @bphan002, thanks for taking the time to work on this issue!

I agree with @kwangric's findings. Based on my investigation, the issue stems from how the "Search" key from filterParams is handled. Functions such as updateCategoryCounter and updateProjectCardDisplayState will throw an undefined error trying to use the 'Search' key.

Looking at what has changed in current-projects.js, it seems that a lot of the code involving the search bar were removed. The best way to fix this would be to bring back some or all of the code used for the search bar. This includes bringing back the use of "AND", "OR", and "-" to refine the search (if this is currently handled somewhere else in your code let me know!).

You can also implement a different approach to handle the errors/ search functionality if you like.

See issue #4136 for the search bar requirements.

[Adastros]

Also, could you update your fork/ branch to include the latest updates from the website repository? It looks like there is a merge conflict with the 'current-projects.js' file. Code involving the tools category of the filter was updated while this pull request was pending review.

[bphan002]

Thank you for the feedback. I'll look over this again. Updated timeline 9/16

…

On Tue, Sep 12, 2023 at 12:38 AM Danny Do ***@***.***> wrote: Also, could you update your fork/ branch to include the latest updates from the website repository? It looks like there is a merge conflict with the 'current-projects.js' file. Code involving the tools category of the filter was updated while this pull request was pending review. — Reply to this email directly, view it on GitHub <#5298 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ATZ7FSRACMKVL4TIS6EL5O3X2AGRDANCNFSM6AAAAAA34UXJPY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[bphan002]

I have re-added the original search function that was removed. I'm not sure why I removed it in the first place, but I think it is working now. The original comments with my first pull request are still an issue I would like guidance on.

Thank you

[Adastros]

Availability (PST): 10 - 11 PM 21SEP23, 3 - 4 PM 23SEP23, 10 - 11:30 PM 25SEP23
Review ETA (PST): End of day 25SEP23

[kwangric]

Thanks for making the changes!

• The code for the message was modified to be reusable for each page with filters.
• The no results message displays correctly on the home, projects, and tooltip page when no cards are displayed.
• The message shows all the filters and searches used.

I think that the parameters names are fine. For the tooltip message, we will want to either add an additional step before passing in the filter params to fix the text. Or change how the page handles the filters so it will work similar to the projects page. This can be done with a future issue I believe.

Overall great work with completing the issue!

[Adastros]

Hi @bphan002, thank you for fixing the searchbar functionality!

• The searchbar functions as expected
• The no results message appears and correctly displays the search terms/ keywords used that led to the no results message
• No issues were found with the no results message at different viewport widths
• works on Firefox and Chrome

One thing that I want to mention is that the original intention of the first task in the issue I created was to move lines 13 - 24 in noResultsMessageFilter.js into a new HTML file in the _includes directory. From there, the the new HTML file would be included in the projects and toolkit page HTML files using Liquid (see lines 48 - 53 in current-guides.html for an example). Reading the issue I created, I should have made that more clear.

That being said, the current implementation does make the no results message a reusable component. Instead of implementing it as mentioned above, it is implemented through JavaScript through noResultsMessageFilter.js, which is included in the relevant HTML page files as an extra script tag.

Since this issue is time sensitive and does make the no results message into a working reusable component, I'm okay with the changes in this pull request.

In regards to parameter names in the no results message on the toolkit page, I agree with @kwangric. That can be addressed in a new, separate issue.

Good job and thank you for working on the issue!