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

frontend: Adds filter feature for app catalog #43

Merged
merged 9 commits into from
Sep 19, 2024
Merged

Conversation

vyncent-t
Copy link
Contributor

@vyncent-t vyncent-t commented May 28, 2024

Add Filter for Verified and Official Apps in App Catalog

Description

Fixes Issue #21

This PR introduces a new filter in the app catalog that allows users to choose whether to display apps based on their verification and official status. By default, the app catalog will only show apps that are verified and official, enhancing security and ensuring that users are installing trusted applications. Users can use the new filter to include apps that are not verified or official if needed.

Changes

  • Added a filter option in the app catalog UI to toggle the display of verified and official apps.
  • Set the default filter state to only show verified and official apps for enhanced security.
  • Implemented functionality to allow users to change the filter settings to include apps that are not verified or official.
  • Updated the app catalog logic to respect the filter settings and dynamically display the appropriate apps.

Verification

  • Verified that the default state of the filter shows only verified and official apps.
  • Tested the filter functionality to ensure users can toggle the display of unverified and unofficial apps.

Screenshots

@vyncent-t
Copy link
Contributor Author

-- the last push

  • have options to filter by verified, official, and cncf (commented out verified and official)
  • sets default search params to be true for verified and official
  • note: any non verified or official charts will not display, to access them turn on commented buttons and toggle

@vyncent-t
Copy link
Contributor Author

currently the issue is that we do not render anything that does not match the search parameters (that are currently set on a default) so charts that are not verified or official do not show up

this is tricky as we use the facets object from the api response to use for our displayed charts and pages navigation

@vyncent-t vyncent-t force-pushed the filter-verified-charts branch 2 times, most recently from 3355c82 to efbcd61 Compare June 5, 2024 17:29
@vyncent-t
Copy link
Contributor Author

vyncent-t commented Jun 13, 2024

Previous push

Fixed

  • fixed issue where toggle would reset on leaving page
  • fixed issue where local storage would not save
  • Added table for additional plugin options
  • Added working switch for "show verified" items

Next

  • need to fix issue where last page would not render correct items (renders empty error message)
  • need to fix or add a redirect link
  • NEED TO REMOVE SETTINGS BUTTONS FROM APP CATALOG VIEW - ONLY THERE FOR PRINTING USE CURRENTLY

@vyncent-t
Copy link
Contributor Author

vyncent-t commented Jun 14, 2024

Previous push

Fixed

  • ! Refactoring the way pages are set and how we find totals for items
  • Before we were taking the total number of all the different kinds in facets and adding them together to get the total of items and not using the number of helm items that are available like on the main site
  • Fixed last page not rendering the items for default
  • Fixed total number of items to accurate helm items

To do

  • Need to fix last page of filter search (currently using filter options like Networking and CNCF does not display the right number of items for that criteria)
  • Need to fix new offset setting

@vyncent-t
Copy link
Contributor Author

Previous push

Fixed

  • ! Refactoring the way pages are set and how we find totals for items

  • Before we were taking the total number of all the different kinds in facets and adding them together to get the total of items and not using the number of helm items that are available like on the main site

  • ! NICE Fixed total number of items to accurate helm items (USING THE HEADER TO FIND THE TOTAL NUMBER OF ITEMS)

  • MOSTLY FIXED Need to fix last page of filter search (DOUBLE LOADS ON OPTIONS CHANGE AND DOES NOT KEEP UP WITH OPTIONS CHANGE)

To do

  • Need to fix new offset setting
  • NEED Fix last page not rendering the items for default

@vyncent-t
Copy link
Contributor Author

Prev push bug fixes

Next

  • Add section for settings using name value table in the extra settings
  • Add hover tool tip for show verified
  • Add a link to the settings page from the catalog page
  • Remove buttons from catalog view
  • Remove CNCF filter options from logic

Done

  • Fixed issue that causes pages to double render on any user action from useEffects
  • Fixed issue where last page of defeat filter search would not render displaying a loop
  • Fixed issue where changing from last page of a view to a new category would cause a "no items" error when items should exist

@vyncent-t
Copy link
Contributor Author

Prev push feat progress

Done

  • Add section for settings using name value table in the extra settings
  • Add hover tool tip for show verified
  • Add a link to the settings page from the catalog page
  • Remove buttons from catalog view
  • Remove CNCF filter options from logic

@vyncent-t
Copy link
Contributor Author

Current screenshots

Main Catalog Page

image
image

When clicking settings link

image
image


App-Catalog Settings Page

image

@vyncent-t vyncent-t marked this pull request as ready for review June 17, 2024 17:43
app-catalog/src/api/charts.tsx Outdated Show resolved Hide resolved
app-catalog/src/api/charts.tsx Outdated Show resolved Hide resolved
app-catalog/src/components/charts/List.tsx Outdated Show resolved Hide resolved
app-catalog/src/components/charts/List.tsx Outdated Show resolved Hide resolved
@vyncent-t
Copy link
Contributor Author

Push note

  • refactoring improvements

@vyncent-t
Copy link
Contributor Author

Note

  • Moving to a draft again, something must have changed in the ArtifactHub API that is not returning headers anymore

app-catalog/src/settings.tsx Outdated Show resolved Hide resolved
app-catalog/src/api/charts.tsx Outdated Show resolved Hide resolved
app-catalog/src/components/charts/List.tsx Outdated Show resolved Hide resolved
app-catalog/src/components/charts/List.tsx Outdated Show resolved Hide resolved
app-catalog/src/settings.tsx Outdated Show resolved Hide resolved
@vyncent-t
Copy link
Contributor Author

moving to WIP until solution for the response header proxy is in

@vyncent-t
Copy link
Contributor Author

vyncent-t commented Jul 19, 2024

@sniok
Notes for the pagination issue:

  • will need to create a solution for both ways to work later

When running 'dev-only-app' our localhost is set to 3000 via ELECTRION_START_URL within the 'dev-only-app' script and we hit a cors wall which does not provide the header for pagination total count

If the app mode is ran with 'npm run dev' the origin is not set to localhost 3000 and cors does not apply
(This can be observed when referencing the Network tab and looking for the 'origin' field under Request Headers)

  • note that the X-Cache reads 'Miss from cloudfront' on this example where the pagination response header is not reached
    image

This means it will not work with 'npm run start' and 'npm run dev only app' but for production and 'npm run dev' it will

Copy link
Contributor

@illume illume left a comment

Choose a reason for hiding this comment

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

I noticed a couple of things that could be in separate commits and left notes for them.

Maybe you know of some other things that can be atomic commits as well?

app-catalog/.gitignore Show resolved Hide resolved
app-catalog/src/components/charts/List.tsx Show resolved Hide resolved
Copy link
Contributor

@illume illume left a comment

Choose a reason for hiding this comment

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

Left some notes about adding these components in separate files and in separate commits.

app-catalog/src/components/charts/List.tsx Outdated Show resolved Hide resolved
@illume
Copy link
Contributor

illume commented Sep 6, 2024

hey hey.

I just noticed the commit messages messages need mending:

  • AppCatalog: Fixes stories for AppCatalogTitle and SettingsLink
  • frontend:

Use app-catalog: and the component(s) as the context.
Use a singular verb to start it. Fix, not Fixes.

app-catalog: ChartsList: Fix verified publisher bug
app-catalog: ChartsList.stories: Add verified publisher states

@illume illume marked this pull request as draft September 6, 2024 18:53
@vyncent-t
Copy link
Contributor Author

hey hey.

I just noticed the commit messages messages need mending:

  • AppCatalog: Fixes stories for AppCatalogTitle and SettingsLink
  • frontend:

Use app-catalog: and the component(s) as the context. Use a singular verb to start it. Fix, not Fixes.

app-catalog: ChartsList: Fix verified publisher bug app-catalog: ChartsList.stories: Add verified publisher states

Should be the right way now!

@illume
Copy link
Contributor

illume commented Sep 6, 2024

Should be the right way now!

Awesome & thanks.

@vyncent-t vyncent-t marked this pull request as ready for review September 10, 2024 11:50
app-catalog/src/components/charts/AppCatalogTitle.tsx Outdated Show resolved Hide resolved
app-catalog/src/components/charts/ChartsList.stories.tsx Outdated Show resolved Hide resolved
app-catalog/src/components/charts/List.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@illume illume left a comment

Choose a reason for hiding this comment

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

👍 thanks

@illume illume dismissed joaquimrocha’s stale review September 19, 2024 18:51

the changes were done

@illume illume merged commit 62ae7ed into main Sep 19, 2024
9 checks passed
@illume illume deleted the filter-verified-charts branch September 19, 2024 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

5 participants