[Core] Quality of Life Updates #140
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
LOTaher
requested review from
bderbs30 and
cherman23
and removed request for
cherman23
ahweigl
reviewed
Dec 18, 2025
Contributor
ahweigl
left a comment
ahweigl
left a comment
There was a problem hiding this comment.
Great stuff!
couple of optional suggestions you could include in this PR:
- make ticket and link here for proj management purposes to track what was done here on the board?
- update readme (idk if it needs anything but Im gonna take a guess no one's looked at it since sept)
- slight pushback on brad's file layout suggestion, I would maybe argue that actions files should go in their respective directories rather than an
/actionsfolder in/app, likesrc/app/(web)/crm/positions/actions.ts, unless you foresee having a lot of actions files that would warrant the folder - why is
/api/positionsingular (position vs positionS)- naming should be consistent imo - might be worth renaming
/api/position/[id]/candidatesto reflect it being candidate pools so it's clearer to distinguish from/api/candidates - abstract
<div> Loading... </div>into a reusable loading state component for across the app? - maybe check if
globals.cssneeds any cleanup
otherwise LGTM 😁
src/lib/components/core/Sidebar.tsx
Outdated
| className="h-7 w-7 overflow-hidden rounded group-data-[collapsible=icon]:mx-auto group-data-[collapsible=icon]:h-5 group-data-[collapsible=icon]:w-5" | ||
| style={ | ||
| !auth.activeOrganization?.logo | ||
| ? { background: '#858C9C' } |
Contributor
There was a problem hiding this comment.
nit: inline color definitions scary, should use a constant defined in globals.css or smth
| alt="Winston Logo" | ||
| width={28} | ||
| height={28} | ||
| className="h-full w-full object-cover object-center brightness-0 invert" |
Contributor
There was a problem hiding this comment.
ooo didnt know this was the way to invert svg colors, neat
bderbs30
reviewed
Dec 22, 2025
| throw new InternalServerException(`Judge0 API error: ${jsonResponse.error}`); | ||
| constructor() { | ||
| this.headers = { | ||
| // eslint-disable-next-line @typescript-eslint/no-non-null-assertion |
Collaborator
There was a problem hiding this comment.
oop is the non null assertion needed here?
bderbs30
reviewed
Dec 22, 2025
| @@ -1,9 +1,9 @@ | |||
| 'use client'; | |||
|
|
|||
| import Image from 'next/image'; | |||
Collaborator
There was a problem hiding this comment.
Are we removing the navbar? It would free up some vertical screen space and I don't think we really utilize the space it takes up and don't foresee future use cases for the space - lmk what you think
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[Core] - Repo QoL Updates
Changes
AppSidebar.tsxandSidebar.tsx(crm)part of our routing and using that as the middleware route/ui/coreand/modalfolders where:uiis for strictly shad componentscoreis for app specific componentsmodalis for modal componentsChecklist
Please go through all items before requesting reviewers:
Closes #142, #141