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

fix(pages): update search results filtering #224

Merged
merged 5 commits into from Mar 19, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
43 changes: 23 additions & 20 deletions client/src/pages/SearchResultPage.vue
Expand Up @@ -42,32 +42,38 @@
</div>

<div data-test="search-results">
<GridContainer
<div
v-for="section in uiShape"
:key="section.header"
:columns="3"
template-rows="auto auto 1fr"
template-rows="auto"
component="section"
data-test="search"
class="p-0 w-full md:max-w-[unset] lg:max-w-[unset]"
class="p-0 w-full"
>
<GridItem class="section-subheading" component="h2" :span-column="3">
<h2 class="section-subheading w-full">
{{ section.header }}
</GridItem>
</h2>

<SearchResultCard
<div
v-for="record in section.records"
:key="record.type"
data-test="search-results-cards"
:data-source="searchResult[record.type]"
/>
</GridContainer>
class="grid pdap-grid-container-column-3 gap-4"
>
<SearchResultCard
v-for="result in searchResult[record.type]"
:key="result.record"
data-test="search-results-cards"
:data-source="result"
/>
</div>
</div>
</div>
</main>
</template>

<script>
import { Button, GridContainer, GridItem } from 'pdap-design-system';
import { Button } from 'pdap-design-system';
import SearchResultCard from '../components/SearchResultCard.vue';
import axios from 'axios';
import pluralize from '../util/pluralize';
Expand All @@ -78,8 +84,6 @@ export default {
components: {
Button,
SearchResultCard,
GridContainer,
GridItem,
},
data: () => ({
count: 0,
Expand Down Expand Up @@ -110,7 +114,12 @@ export default {

// Format results into object keyed by record_type
const resultFormatted = res.data.data.reduce((acc, cur) => {
return { ...acc, [cur.record_type]: cur };
return {
...acc,
[cur.record_type]: Array.isArray(acc[cur.record_type])
? [...acc[cur.record_type], cur]
: [cur],
};
Comment on lines +115 to +120
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@maxachis see this part of the code wrt bug fix for #217. The previous logic was keying an object by record_type but forgot to take into account that there might be multiple records of the same type. To fix this bug, we're instead assigning an array to each of those keys, to accommodate multiple values of each key. The updates to the template above are in service of this change (as well as some general cleanup, as we're moving away from the GridContainer and GridItem components).

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, that would make sense. So regardless of whether there's one record or multiple, it is formatted into an array, simplifying the logic on the backend.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that would make sense. So regardless of whether there's one record or multiple, it is formatted into an array, simplifying the logic on the backend.

Yeah @maxachis I think @mbodeantor and I are aligned on the idea that the API should be as simple as possible, and any complex manipulation required by UX or design requirements should be handled by the client.

}, {});

// Modify ui shape object to exclude any sections / data sources that do not have records returned by API
Expand All @@ -135,9 +144,3 @@ export default {
},
};
</script>

<style scoped>
.section-subheading {
@apply mt-4;
}
</style>