-
Notifications
You must be signed in to change notification settings - Fork 114
feat(hub): logs #2364
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(hub): logs #2364
Conversation
Deploying rivet-hub with
|
Latest commit: |
a8959bc
|
Status: | ✅ Deploy successful! |
Preview URL: | https://998ec78c.rivet-hub-7jb.pages.dev |
Branch Preview URL: | https://04-15-feat-hub-functions.rivet-hub-7jb.pages.dev |
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
How to use the Graphite Merge QueueAdd the label merge-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Deploying rivet with
|
Latest commit: |
a8959bc
|
Status: | ✅ Deploy successful! |
Preview URL: | https://bb87a6e5.rivet.pages.dev |
Branch Preview URL: | https://04-15-feat-hub-functions.rivet.pages.dev |
6b80b50
to
6973c98
Compare
6973c98
to
6d6eeb8
Compare
6d6eeb8
to
d569321
Compare
Graphite Automations"Test" took an action on this PR • (04/17/25)1 assignee was added to this PR based on Kacper Wojciechowski's automation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This PR introduces a comprehensive functions view and route management system to the Rivet hub application, with significant UI and architectural improvements.
- Added new functions view in
/frontend/apps/hub/src/routes/_authenticated/_layout/projects/$projectNameId/environments/$environmentNameId._v2/functions.tsx
with virtualized table for efficient rendering of function invocations - Implemented route management dialogs in
/frontend/apps/hub/src/domains/project/components/dialogs/create-route-dialog.tsx
andedit-route-dialog.tsx
with form validation - Added new v2 layout system across multiple components with collapsible sidebar and improved error handling
- Added table utilities in
/frontend/packages/components/src/lib/table.ts
for flexible column sizing and virtualization support - Introduced route mutations (patch, create, delete) in
/frontend/apps/hub/src/domains/project/queries/actors/mutations.ts
with proper query invalidation
💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!
27 file(s) reviewed, 34 comment(s)
Edit PR Review Bot Settings | Greptile
frontend/apps/hub/src/domains/project/components/dialogs/create-route-dialog.tsx
Outdated
Show resolved
Hide resolved
frontend/apps/hub/src/domains/project/components/dialogs/create-route-dialog.tsx
Outdated
Show resolved
Hide resolved
import { | ||
useMatches, | ||
useMatchRoute, | ||
useRouterState, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: useRouterState is imported but never used in the code
useRouterState, | |
useMatches, | |
useMatchRoute, |
frontend/apps/hub/src/domains/project/components/dialogs/edit-route-dialog.tsx
Outdated
Show resolved
Hide resolved
frontend/apps/hub/src/domains/project/components/dialogs/edit-route-dialog.tsx
Outdated
Show resolved
Hide resolved
"px-8 w-full h-full": | ||
layout === "full" || layout === "actors", | ||
"px-4 w-full h-full": layout === "v2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: 'onboarding' layout is missing from this condition but present in main PageLayout component
"h-full": | ||
layout === "full" || | ||
layout === "onboarding" || | ||
layout === "actors", | ||
layout === "actors" || | ||
layout === "v2", | ||
})} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider extracting layout conditions into a separate utility function to improve maintainability as more layouts are added
let calculatedSize = 100; | ||
calculatedSize = Math.floor(totalAvailableWidth / totalIsGrow); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Unnecessary initialization of calculatedSize to 100 since it's immediately overwritten on the next line
let calculatedSize = 100; | |
calculatedSize = Math.floor(totalAvailableWidth / totalIsGrow); | |
const calculatedSize = Math.floor(totalAvailableWidth / totalIsGrow); |
if (!column.size) { | ||
if (!column.meta?.isGrow) { | ||
let calculatedSize = 100; | ||
if (column?.meta?.widthPercentage) { | ||
calculatedSize = | ||
column.meta.widthPercentage * totalWidth * 0.01; | ||
} else { | ||
calculatedSize = totalWidth / columns.length; | ||
} | ||
|
||
const size = getSize( | ||
calculatedSize, | ||
column.maxSize, | ||
column.minSize, | ||
); | ||
|
||
column.size = size; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Column size calculation should happen even if column.size exists but isGrow is true, otherwise growing columns won't resize properly when window changes
let totalAvailableWidth = totalWidth; | ||
let totalIsGrow = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: totalAvailableWidth could become negative if sum of minimum widths exceeds total width, leading to incorrect calculations
74caf46
to
dca2d32
Compare
1f203f6
to
7e9c119
Compare
7d42682
to
3c337f5
Compare
7e9c119
to
b08ab09
Compare
3c337f5
to
544d442
Compare
b08ab09
to
9ab3977
Compare
1ea1fde
to
0c8f699
Compare
9ab3977
to
1d4d4f6
Compare
f380d1b
to
c429793
Compare
596e93d
to
a8959bc
Compare
1d4d4f6
to
ee96ff9
Compare
a8959bc
to
7de6870
Compare
Merge activity
|
<!-- Please make sure there is an issue that this PR is correlated to. --> ## Changes <!-- If there are frontend changes, please include screenshots. -->
<!-- Please make sure there is an issue that this PR is correlated to. --> ## Changes <!-- If there are frontend changes, please include screenshots. -->
Changes