PAC-1650-filters data persisted, version tabs hidden, version based r… #2792
Conversation
nage1234
requested review from
karl-cardenas-coding,
lennessyy and
caroldelwing
and removed request for
a team
…precated, disabled, removed additionalFilters
| @@ -12,22 +12,16 @@ interface PackFiltersProps { | |||
| } | |||
|
|
|||
| export default function PacksFilters({ categories, registries, setSelectedSearchFilters, selectedFilters }: PackFiltersProps) { | |||
| const additionalFiltersProps: string[] = [ | |||
| const sourceList: string[] = [ | |||
There was a problem hiding this comment.
these options are static. can be moved outside of this. I'd be careful to how labels are capitalized, so include value and label
plugins/packs-integrations.js
Outdated
| @@ -204,10 +206,12 @@ async function pluginPacksAndIntegrationsData(context, options) { | |||
| const packUrl = "v1/packs/"; | |||
| const packMDMap = new Map(); | |||
| let apiPacksData = []; | |||
| const packuid = mappedRepos?.[0]?.uid; | |||
There was a problem hiding this comment.
preferedRegistryUid
plugins/packs-integrations.js
Outdated
| const promises = packDataArr.map((packData) => { | ||
| packMDMap[packData.spec.name] = packData; | ||
| const cloudType = packData.spec.cloudTypes.includes("all") ? "aws" : packData.spec.cloudTypes[0]; | ||
| const url = `${packUrl}${packData.spec.name}/registries/${packData.spec.registries[0].uid}?cloudType=${cloudType}&layer=${packData.spec.layer}`; | ||
| const packUid = packData.spec.registries.find((registry) => registry.uid === packuid); |
plugins/packs-integrations.js
Outdated
| const promises = packDataArr.map((packData) => { | ||
| packMDMap[packData.spec.name] = packData; | ||
| const cloudType = packData.spec.cloudTypes.includes("all") ? "aws" : packData.spec.cloudTypes[0]; | ||
| const url = `${packUrl}${packData.spec.name}/registries/${packData.spec.registries[0].uid}?cloudType=${cloudType}&layer=${packData.spec.layer}`; | ||
| const packUid = packData.spec.registries.find((registry) => registry.uid === packuid); | ||
| const url = `${packUrl}${packData.spec.name}/registries/${packUid?.uid || packData.spec.registries[0].uid}?cloudType=${cloudType}&layer=${packData.spec.layer}`; |
There was a problem hiding this comment.
packUid?.uid you already have the UID from mappedRepos?.[0]?.uid;
Why create packUid once more?
There was a problem hiding this comment.
@codrin-iftimie say, i have given A, B in the docusaurus config file. So, our conclusion is to use A as the preferred registry to fetch the details of the pack
There can be another scenario, while traversing the list of packs, some packs may be part of registry B, not A. So, this could end up in error if i fetch the details of those packs which are not part of A. Thats the reason why i am creating another packUid here.. i will update the proper comments for this for future correspondence
| <InfoCircleOutlined className={styles.infoIcon} /> | ||
| {"Info"} | ||
| </div> | ||
| <div className={styles.content}>{infoContent[0]}</div> |
There was a problem hiding this comment.
why show only one if you go in all of the trouble of creating this array?
There was a problem hiding this comment.
let me do simply with the conditional operators.. as they are simple booleans
| ...selectedSearchFilters | ||
| })); | ||
| }; | ||
| localStorage.setItem("selectedFilters", JSON.stringify(updatedFilters)); |
There was a problem hiding this comment.
Better make this a bit more specific "packList.selectedFilters" and make it a constant (to be used in both places when you set or get the value in the local storage)
and include the search value in these filters. There is no need for two different storage items
There was a problem hiding this comment.
search is still a filter
| const packUid = packData.spec.registries.find((registry) => registry.uid === packuid); | ||
| const url = `${packUrl}${packData.spec.name}/registries/${packUid?.uid || packData.spec.registries[0].uid}?cloudType=${cloudType}&layer=${packData.spec.layer}`; | ||
| const preferredRegistry = packData.spec.registries.find((registry) => registry.uid === preferredRegistryUid); | ||
| //there is a scenario where the pack is not part of preferred registry, in that case, the item of the pack registries is sent to API request |
There was a problem hiding this comment.
then lets make this obvious.
const hasPreferredRegistry = packData.spec.registries.some((registry) => registry.uid === preferredRegistryUid);
let packRegistryUid = packData.spec.registries[0].uid
if (hasPreferredRegistry) {
packRegistryUid = preferedRegistryUid
}
const url = `${packUrl}${packData.spec.name}/registries/${packRegistryUid}?cloudType=${cloudType}&layer=${packData.spec.layer}`;| const infoContent = packData.disabled ? "This pack is currently disabled." : ( | ||
| packData.deprecated ? "This pack is deprecated." : undefined | ||
| ); |
There was a problem hiding this comment.
Ternary operators (multiple) makes this hard to read.
| @@ -212,13 +210,13 @@ export default function PacksReadme() { | |||
| </div> | |||
| </div> | |||
| </div> | |||
| {infoContent.length > 0 && ( | |||
| {infoContent && ( | |||
There was a problem hiding this comment.
{infoContent ? (// etc) : null}
| if (filters) { | ||
| setSelectedFilters(JSON.parse(filters)); | ||
| const { selectedFilters, searchValue } = JSON.parse(filters); |
There was a problem hiding this comment.
JSON.parse can fail
…outing
Describe the Change
This PR ....
Changed Pages
💻 Add Preview URL for Page
Jira Tickets
🎫 Jira Ticket
Backports
Can this PR be backported?