feat: add empty state when no jobs match filters - closes #30#328
feat: add empty state when no jobs match filters - closes #30#328aaniya22 wants to merge 3 commits into
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe job browsing page now evaluates whether all three job sources (internal, external/curated, and scraped/partners) are simultaneously empty, and displays a unified global empty state with an optional "Clear filters" button instead of showing an internal-only empty state. The internal jobs section is suppressed when this global empty condition is true. ChangesGlobal Empty State for Job Browsing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
client/src/module/student/jobs/JobBrowsePage.tsx (1)
545-546: ⚡ Quick winReplace arbitrary Tailwind text-size classes with standard scale classes.
Changed lines include
text-[10px]andtext-[11px]; use canonical scale tokens (text-xs, etc.) to match project Tailwind rules.As per coding guidelines, "Do not use arbitrary bracket sizes like
text-[17px], use standard scale classes instead".Also applies to: 559-559, 585-585, 598-598, 611-611, 618-618
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/module/student/jobs/JobBrowsePage.tsx` around lines 545 - 546, In JobBrowsePage (JSX in the JobBrowsePage component) replace the arbitrary Tailwind sizes `text-[10px]` and `text-[11px]` used in className strings (occurrences around the inline-flex span and the other noted spots) with the project’s canonical scale tokens (e.g. text-xs or text-sm as appropriate) so they follow the standard Tailwind scale; search for `text-[10px]` / `text-[11px]` in JobBrowsePage.tsx and swap each with the closest standard class (prefer text-xs for these small labels) preserving the rest of the className values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/src/module/student/jobs/JobBrowsePage.tsx`:
- Around line 249-253: The empty-state calculation for allEmpty (using
filteredExtJobs, scrapedJobs and data?.jobs) only checks the internal isLoading,
so it can report "No jobs" while external queries are still pending; update the
logic that defines allEmpty to also require external/scraped query completion
(e.g., include their loading flags such as isFilteredExtLoading /
isScrapedLoading or use their isFetching/isLoading statuses) before deciding
empty, or create a combined loading flag (e.g., const anyLoading = isLoading ||
isFilteredExtLoading || isScrapedLoading) and use !anyLoading && ... when
computing allEmpty; ensure you reference the existing symbols filteredExtJobs,
scrapedJobs, isLoading, and data?.jobs when making the change.
- Around line 528-536: The Clear filters button is misleading because clearAll
does not reset the hideExpired state and hasFilters doesn't include hideExpired,
so when hideExpired is the only active filter the button can disappear or not
fully reset; update the hasFilters boolean logic (wherever it's computed) to
include the hideExpired flag and modify the clearAll handler (function clearAll)
to reset hideExpired to its default value (same reset behavior as other filters)
so the button is visible whenever any filter including hideExpired is active and
clicking it truly clears all filters.
- Around line 529-535: Replace the raw <button> in JobBrowsePage.tsx that calls
clearAll with the shared Button component from
client/src/components/ui/button.tsx: import Button and swap the element using
the Button API (pass onClick={clearAll}, remove the custom className and use
props for variant and size such as variant="mono" or "ghost" and size="sm" to
preserve styling), keep the <X /> icon and "Clear filters" text as children, and
ensure any accessibility attributes (aria-label) from the original are
preserved.
---
Nitpick comments:
In `@client/src/module/student/jobs/JobBrowsePage.tsx`:
- Around line 545-546: In JobBrowsePage (JSX in the JobBrowsePage component)
replace the arbitrary Tailwind sizes `text-[10px]` and `text-[11px]` used in
className strings (occurrences around the inline-flex span and the other noted
spots) with the project’s canonical scale tokens (e.g. text-xs or text-sm as
appropriate) so they follow the standard Tailwind scale; search for
`text-[10px]` / `text-[11px]` in JobBrowsePage.tsx and swap each with the
closest standard class (prefer text-xs for these small labels) preserving the
rest of the className values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 900761dc-b4d8-4038-80a8-2090ac8042dc
📒 Files selected for processing (1)
client/src/module/student/jobs/JobBrowsePage.tsx
| <button | ||
| type="button" | ||
| onClick={clearAll} | ||
| className="inline-flex items-center gap-2 px-4 py-2.5 rounded-md text-xs font-bold bg-stone-900 dark:bg-stone-50 text-stone-50 dark:text-stone-900 hover:bg-stone-800 dark:hover:bg-stone-200 transition-colors border-0 cursor-pointer" | ||
| > | ||
| <X className="w-3.5 h-3.5" /> Clear filters | ||
| </button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use the shared Button component for the new CTA button.
This new button should use the reusable UI Button for consistency and variant control.
Proposed fix
- <button
- type="button"
- onClick={clearAll}
- className="inline-flex items-center gap-2 px-4 py-2.5 rounded-md text-xs font-bold bg-stone-900 dark:bg-stone-50 text-stone-50 dark:text-stone-900 hover:bg-stone-800 dark:hover:bg-stone-200 transition-colors border-0 cursor-pointer"
- >
+ <Button
+ type="button"
+ onClick={clearAll}
+ variant="primary"
+ size="md"
+ className="inline-flex items-center gap-2"
+ >
<X className="w-3.5 h-3.5" /> Clear filters
- </button>
+ </Button>As per coding guidelines, "Use the reusable Button component from client/src/components/ui/button.tsx for all new buttons with variants (primary, secondary, mono, ghost, danger), modes (button, icon, link), and sizes (sm, md, lg)".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/src/module/student/jobs/JobBrowsePage.tsx` around lines 529 - 535,
Replace the raw <button> in JobBrowsePage.tsx that calls clearAll with the
shared Button component from client/src/components/ui/button.tsx: import Button
and swap the element using the Button API (pass onClick={clearAll}, remove the
custom className and use props for variant and size such as variant="mono" or
"ghost" and size="sm" to preserve styling), keep the <X /> icon and "Clear
filters" text as children, and ensure any accessibility attributes (aria-label)
from the original are preserved.
|
please review and merge this pr |
|
Can you resolve the conflict |
What's changed
Related issue
Closes #30
Summary by CodeRabbit