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.

Sign up for GitHub

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

Jump to bottom

ffxiv-export-url-endings #490

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

ffxiv-export-url-endings #490

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bccd397
ffxiv-export-url-endings
cohenaj194 Sep 1, 2024
3069ba4
Update queries.world-comparison.tsx
cohenaj194 Sep 1, 2024
a7684b9
adding ai recommendation
cohenaj194 Sep 1, 2024
eb98ae7
Merge branch 'master' into ffxiv-export-url-endings
cohenaj194 Sep 13, 2024
31f9626
Merge branch 'master' into ffxiv-export-url-endings
cohenaj194 Oct 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 51 additions & 2 deletions app/routes/queries.world-comparison.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useActionData, useNavigation } from '@remix-run/react'
import { useActionData, useLoaderData, useNavigation } from '@remix-run/react'
import { ContentContainer, PageWrapper, Title } from '~/components/Common'
import SmallFormContainer from '~/components/form/SmallFormContainer'
import type { ActionFunction } from '@remix-run/cloudflare'
Expand All @@ -20,6 +20,10 @@ import {
} from '@heroicons/react/outline'
import { WorldList } from '~/utils/locations/Worlds'
import TitleTooltip from '~/components/Common/TitleTooltip'
import {
getActionUrl,
handleSearchParamChange
} from '~/utils/urlSeachParamsHelpers'

const pathHash: Record<string, string> = {
hqOnly: 'High Quality Only',
Expand All @@ -46,6 +50,42 @@ export const links: LinksFunction = () => [
}
]

// Define default form values
const defaultFormValues = {
itemIds: '',
exportServers: '',
hqOnly: 'false'
}

// Define input schema for validation
const inputSchema = z.object({
itemIds: z.string().min(1),
exportServers: z.string().min(1),
hqOnly: z.optional(z.string())
})

// Loader function to handle URL parameters
export const loader: LoaderFunction = async ({ request }) => {
const params = new URL(request.url).searchParams

const values = {
itemIds: params.get('itemIds') || defaultFormValues.itemIds,
exportServers:
params.get('exportServers') || defaultFormValues.exportServers,
hqOnly: params.get('hqOnly') || defaultFormValues.hqOnly
}

const validParams = inputSchema.safeParse(values)
if (!validParams.success) {
return json({
exception: `Missing: ${validParams.error.issues
.map(({ path }) => path.join(', '))
.join(', ')}`
})
}
return json(validParams.data)
}

const sortByPrice =
(desc: boolean) => (first: { price: number }, second: { price: number }) => {
if (first.price === second.price) {
Expand Down Expand Up @@ -101,13 +141,21 @@ const Index = () => {
const results = useActionData<
ItemServerComparisonList | { exception: string } | {}
>()
const loaderData = useLoaderData<typeof defaultFormValues>()

const [modal, setModal] = useState<'exportServers' | 'items' | null>(null)
const [state, setState] = useState<{
items: string[]
exportServers: string[]
}>({ items: [], exportServers: [] })

// // One AI recommendation, but it didnt solve the issue. It keeps saying "Error: Cannot read properties of undefined (reading 'split')"
// const [state, setState] = useState({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

AI recommendation, this might be right, but it does not solve the whole problem.

// items: loaderData.itemIds ? loaderData.itemIds.split(',') : [], // Add a check for undefined
// exportServers: loaderData.exportServers ? loaderData.exportServers.split(',') : [], // Add a check for undefined
// hqOnly: loaderData.hqOnly === 'false',
// })

const onSubmit = (e: React.MouseEvent<HTMLButtonElement>) => {
if (transition.state === 'submitting') {
e.preventDefault()
Expand Down Expand Up @@ -137,7 +185,8 @@ const Index = () => {
onClick={onSubmit}
loading={transition.state === 'submitting'}
disabled={transition.state === 'submitting'}
error={error}>
error={error}
action={getActionUrl('/queries/world-comparison', state)}>
<div className="pt-4">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@coderabbitai this does change the url of the page but loading something like http://localhost:8788/queries/world-comparison?items=21793&exportServers=Adamantoise%2CHalicarnassus does not alter the forms in the webpage.

Can you look through other past pull requests to see how we fixed this before and recommend a fix?

Copy link
Contributor

Choose a reason for hiding this comment

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

@cohenaj194, it seems that there are no past pull requests that directly address the issue of populating form fields based on URL parameters. However, I can recommend a solution based on the current context.

The issue likely stems from the form fields not being initialized with the data returned by the loader function. To fix this, you can use the useLoaderData hook to set the initial state of the form fields when the page loads.

Here's an example of how you can implement this:

const Index = () => {
  const transition = useNavigation();
  const results = useActionData<
    ItemServerComparisonList | { exception: string } | {}
  >();
  const loaderData = useLoaderData<typeof defaultFormValues>();

  const [state, setState] = useState({
    items: loaderData.itemIds.split(','),
    exportServers: loaderData.exportServers.split(','),
    hqOnly: loaderData.hqOnly === 'true',
  });

  // Rest of the component code...
};

In this example, the state is initialized with the values returned by the loader function, ensuring that the form fields are populated correctly when the page loads.

Let me know if you need further assistance with this!

Analysis chain
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search through past pull requests for keywords related to URL parameters and form population.
gh pr list --search "URL parameters form population" --json number,title,body --limit 10

Length of output: 93

Script:

#!/bin/bash
# Perform a broader search for past pull requests using more general keywords related to query parameters and form data.
gh pr list --search "query parameters form data" --json number,title,body --limit 10

Length of output: 727

Learnings added 
Learnt from: cohenaj194
PR: ff14-advanced-market-search/saddlebag-with-pockets#490
File: app/routes/queries.world-comparison.tsx:182-182
Timestamp: 2024-09-01T16:45:03.690Z
Learning: When dealing with form fields that need to be populated based on URL parameters, ensure that the initial state of the form fields is set using the data returned by the `loader` function via the `useLoaderData` hook.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Learnings used 
Learnt from: cohenaj194
PR: ff14-advanced-market-search/saddlebag-with-pockets#0
File: :0-0
Timestamp: 2024-06-09T21:34:49.875Z
Learning: User: cohenaj194 prefers detailed, actionable responses that directly address the code issues with specific solutions and examples.

<div className="sm:px-4 flex flex-col gap-4">
<div className="flex flex-col max-w-full relative">
Expand Down
Loading