Skip to content

implemented company filtering #195

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

Merged
merged 7 commits into from
Apr 10, 2025
Merged

implemented company filtering #195

merged 7 commits into from
Apr 10, 2025

Conversation

gpalmer27
Copy link
Contributor

Description

Added a filter bar to the companies page so that users can filter based on industry and location. Also fixed a bug with the width of the company page being greater than 100%.

Motivation and Context

Helps users narrow down what companies they are looking at.

Closes #190

How has this been tested?

Pressed the filter buttons and they worked.

Screenshots (if appropriate):

Screenshot 2025-04-06 at 10 29 19 PM

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • [ x] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Database migration
    • Ran pnpm db:generate and verified generated SQL migration files in packages/db/drizzle

Checklist:

  • [ x] My code follows the code style of this project.
  • [x ] I have moved the ticket to "In Review"
  • [ x] I have added reviewers to this PR
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link

vercel bot commented Apr 7, 2025

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

Name Status Preview Comments Updated (UTC)
cooper ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 8, 2025 10:27pm
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
cooper-auth ⬜️ Skipped (Inspect) Apr 8, 2025 10:27pm
cooper-docs ⬜️ Skipped (Inspect) Apr 8, 2025 10:27pm

@gpalmer27 gpalmer27 requested a review from banushi-a April 7, 2025 02:30
Copy link
Contributor

@banushi-a banushi-a left a comment

Choose a reason for hiding this comment

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

like I said we probably don't want to use the locations . list api since there are about 13k rows in that table right now

const [selectedLocation, setSelectedLocation] = useState<string | undefined>(
location?.city,
);
const { data: locations = [] } = api.location.list.useQuery();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is causing the extreme latency which I'm seeing on the preview deployment. I think Tracy has solved this problem though. Spend some time looking through review-section.tsx in the form to see how Tracy implemented a Locations combobox.
This should help with the problems we are facing right now. Once we get that figured out I can review (it is too slow to test right now)

render={({ field }) => (
<FormItem className="col-span-5 lg:col-span-2">
<FormControl>
<Select
Copy link
Contributor

Choose a reason for hiding this comment

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

We will probably need to replace this select with the Combobox mentioned above.

@@ -51,15 +51,18 @@ export default function ComboBox({
const styleVariant =
variant === "form"
? "flex h-16 w-full rounded-md border-[3px] border-cooper-blue-600 bg-white px-3 py-2 text-xl font-normal ring-offset-background file:border-0 file:bg-transparent file:text-sm file:font-medium placeholder:text-muted-foreground focus-visible:outline-none focus-visible:ring-0 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50"
: "";
: variant === "filtering"
? "w-[340px] h-12 rounded-none border-2 border-l-0 border-t-0 border-[#9A9A9A] text-lg placeholder:opacity-50 focus:ring-0 active:ring-0 lg:rounded-md lg:border-2 py-0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of 340px can we use 340/16 ~ 21rem

Also for the border let's use border-[0.75px] to be consistent with the Figma and all the other borders we use throughout the app.

If possible, would you also be able to make it so that the placeholder text is the same color as the border?

@@ -94,13 +106,16 @@ export default function SearchFilter({
<Form {...form}>
<form
onSubmit={form.handleSubmit(onSubmit)}
className={cn("w-[100vw]", searchClassName)}
className={cn("w-[90vw]", searchClassName)}
Copy link
Contributor

Choose a reason for hiding this comment

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

Right now the Clear button doesn't align with the right hand side of the screen like it does with the figma

Screenshot 2025-04-08 at 10 15 23 AM

One solution I found (which also preserves the old styles) is

      <form
        onSubmit={form.handleSubmit(onSubmit)}
        className={cn(
          "w-[98vw] border border-blue-700",
          searchType === "COMPANIES" && "w-full",
          searchClassName,
        )}
      >

Let me know if the w-[98vw] still gives you that weird issue we encountered where the width of the companies page is too much

@@ -19,7 +19,7 @@ export function SimpleSearchBar() {
const newLocal =
"h-12 border border-l-0 border-cooper-blue-800 text-lg text-cooper-blue-600 placeholder:text-cooper-blue-600 rounded-r-lg rounded-l-none";
return (
<div className="flex w-full rounded-lg">
<div className="flex w-full rounded-lg w-full">
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like there are two w-full here so we can remove the second

)}
/>
<Button
className="bg-white hover:bg-white hover:text-[#9A9A9A] border-white text-black"
Copy link
Contributor

Choose a reason for hiding this comment

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

Here we can make the text color here the same as the border color you used above to match the figma

@vercel vercel bot temporarily deployed to Preview – cooper-docs April 8, 2025 22:26 Inactive
@vercel vercel bot temporarily deployed to Preview – cooper-auth April 8, 2025 22:26 Inactive
Copy link
Contributor

@banushi-a banushi-a left a comment

Choose a reason for hiding this comment

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

YAYYYY LET"S GOOOOOO

@gpalmer27 gpalmer27 merged commit 8198869 into main Apr 10, 2025
9 checks passed
@gpalmer27 gpalmer27 deleted the company-filter branch April 10, 2025 16:39
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.

(Develop) Update Company Page
2 participants