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

feat(app): add filtering questions #407

Merged
merged 15 commits into from
Dec 13, 2022
Merged

Conversation

AdiPol1359
Copy link
Contributor

Fixes #405

@vercel
Copy link

vercel bot commented Dec 12, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
devfaq ✅ Ready (Inspect) Visit Preview Dec 13, 2022 at 3:43PM (UTC)

@github-actions
Copy link

github-actions bot commented Dec 12, 2022

📦 Next.js Bundle Analysis

This analysis was generated by the next.js bundle analysis action 🤖

🎉 Global Bundle Size Decreased

Page Size (compressed)
global 79.01 KB (-1 B)
Details

The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!

apps/app/src/app/questions/[technology]/[page]/page.tsx Outdated Show resolved Hide resolved
import { ChangeEvent, useState } from "react";
import { DEFAULT_ORDER_QUERY } from "../../lib/order";

export const useQuestionsHeader = () => {
Copy link
Member

Choose a reason for hiding this comment

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

Czy to na pewno dobra nazwa na hooka, który pozwala na sortowanie pytań? Raczej nie.
Może useQuestionsSortBy, który zwraca {orderBy, order i setOrderBy} ?

setValue(value);

if (pathname) {
router.push(`${pathname}?sortBy=${value}`);
Copy link
Member

Choose a reason for hiding this comment

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

Powinniśmy zachować pozostałe query params jeśli jakieś były. Teraz nadpisujemy wszystkie.
Możemy zrobić do tego osobnego hooka w stylu:

const {mergeQueryParams} = useDevFAQRouter();
// …
mergeQueryParams({sortBy: value})

variant="default"
defaultValue={DEFAULT_ORDER_QUERY}
onChange={handleSelectChange}
className="ml-3"
Copy link
Member

Choose a reason for hiding this comment

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

Brakuje value={orderBy}


const setOrderBy = (order: string) => {
if (pathname) {
router.push(`${pathname}?${mergeQueryParams({ sortBy: order })}`);
Copy link
Member

Choose a reason for hiding this comment

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

Czemu tego wszystkiego nie robi mergeQueryParams?


const setOrderBy = (order: string) => {
if (pathname) {
router.push(`${pathname}?${mergeQueryParams({ sortBy: order })}`);
Copy link
Member

Choose a reason for hiding this comment

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

A gdzie walidacja tego order ?

return orderBy.includes(data);
};

export const validateOrder = (data: unknown): data is typeof order[number] => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export const validateOrder = (data: unknown): data is typeof order[number] => {
export const validateOrder = (data: string): data is typeof order[number] => {


export const DEFAULT_ORDER_QUERY = "acceptedAt*desc";

export const validateOrderBy = (data: unknown): data is typeof orderBy[number] => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export const validateOrderBy = (data: unknown): data is typeof orderBy[number] => {
export const validateOrderBy = (data: string): data is typeof orderBy[number] => {

}) {
const page = parseInt(params.page);
const [orderBy, order] = (searchParams?.sortBy || DEFAULT_ORDER_QUERY).split("*");
Copy link
Member

Choose a reason for hiding this comment

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

To i walidacja nie powinny być w komponencie.

@typeofweb typeofweb merged commit 03b6dd4 into develop Dec 13, 2022
@typeofweb typeofweb deleted the 405-filtering-questions branch December 13, 2022 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add filtering questions
2 participants