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: Remove default values from URL #666

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

murilx
Copy link
Contributor

@murilx murilx commented Dec 10, 2024

  • Added the search middleware stripSearchParams to remove search params with default values from the URL
  • Changed from options to use route instead of index e.g from /tree/$treeId/ to /tree/$treeId
  • Added some default clauses to zod types to make the default values of search params be returned when they are stripped from the URL. Without this clause the returned value would be undefined

Closes #656

How to test

Navigate through the app, verifying that default values aren't being shown in the URL while keeping the same behaviour. Also note that when changing something, the search params goes back to the URL.

@murilx murilx force-pushed the feat/remove-default-values-url branch 2 times, most recently from 525eb27 to aa6f0b3 Compare December 11, 2024 13:01
@murilx murilx self-assigned this Dec 11, 2024
@murilx murilx marked this pull request as ready for review December 11, 2024 13:11
@@ -146,7 +163,8 @@ export const zTreeInformation = z
commitName: z.string().optional().catch(''),
Copy link
Collaborator

@WilsonNet WilsonNet Dec 11, 2024

Choose a reason for hiding this comment

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

another ticket for you, create a ticket to remove parameters that does not need to be in the URL

the URL is to remember user choices, this can be inferred from the sql query response

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done #671

const zHardwareSchema = z.object({
intervalInDays: makeZIntervalInDays(DEFAULT_HARDWARE_INTERVAL_IN_DAYS),
hardwareSearch: z.string().optional().catch(undefined),
hardwareSearch: z.string().optional().catch(''),
Copy link
Collaborator

Choose a reason for hiding this comment

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

from what I'm understanding with stripSerach poarameters you don't need .optional anymore

export const RootSearchSchema = z.object({
intervalInDays: makeZIntervalInDays(DEFAULT_TIME_SEARCH),
treeSearch: z.string().optional().catch(''),
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto about optional

const { isLoading, data, error } = useTreeDetails({
treeId: treeId ?? '',
filter: reqFilter,
});

const { tableFilter, diffFilter } = useSearch({
from: '/tree/$treeId/',
from: '/tree/$treeId',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you change this?
/ is suppose to be the leaf, and that is the more exact place in this current route

@@ -35,14 +35,14 @@ interface TestsTabProps {
}

const TestsTab = ({ reqFilter }: TestsTabProps): JSX.Element => {
const { treeId } = useParams({ from: '/tree/$treeId/' });
const { treeId } = useParams({ from: '/tree/$treeId' });
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto about leaf

Copy link
Collaborator

@MarceloRobert MarceloRobert left a comment

Choose a reason for hiding this comment

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

From my tests, it seems that the buildDetails page still receives an empty diffFilter, and the hardwareDetails page is behaving weirdly, I can't deselect trees and it seems laggy

@murilx murilx force-pushed the feat/remove-default-values-url branch from aa6f0b3 to 811eefb Compare December 11, 2024 19:55
- Added the search middleware `stripSearchParams` to remove search
  params with default values from the URL
- Changed `from` options to use route instead of index e.g
  from `/tree/$treeId/` to `/tree/$treeId`
- Added some default clauses to zod types to make the default values of
  search params be returned when they are stripped from the URL. Without
  this clause the returned value would be undefined

Closes #656
@murilx murilx force-pushed the feat/remove-default-values-url branch from 811eefb to 8c661c2 Compare December 11, 2024 20:14
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.

remove default values from URL
3 participants