-
Notifications
You must be signed in to change notification settings - Fork 0
feature - left nav match #217
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
Conversation
WalkthroughThe pull request introduces significant updates to the frontend navigation structure. The changes modify the Changes
Sequence DiagramsequenceDiagram
participant User
participant NavigationBar
participant EntityConfig
User->>NavigationBar: Interact with navigation
NavigationBar->>EntityConfig: Retrieve all entities
EntityConfig-->>NavigationBar: Return full entity list
NavigationBar->>User: Display comprehensive navigation
Possibly related PRs
Suggested reviewers
Poem
Warning Review ran into problems🔥 ProblemsGitHub Actions: Resource not accessible by integration - https://docs.github.com/rest/actions/workflow-runs#list-workflow-runs-for-a-repository. Please grant the required permissions to the CodeRabbit GitHub App under the organization or repository settings. Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/lib/components/NavigationBar/NavigationBar.svelte (1)
162-162
: Use appropriate icon for Support button.IconDocumentText is reused from docs button. Consider using a support-specific icon.
- <IconDocumentText class="text-muted-icon-neutral" /> + <IconQuestionMarkCircle class="text-muted-icon-neutral" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (3)
frontend/src/lib/components/NavigationBar/NavigationBar.svelte
(3 hunks)frontend/src/lib/types/Entity/Entity.ts
(2 hunks)frontend/src/routes/+layout.svelte
(1 hunks)
🔇 Additional comments (2)
frontend/src/routes/+layout.svelte (1)
26-26
: LGTM!Passing full entityConfig enables complete navigation structure.
frontend/src/lib/components/NavigationBar/NavigationBar.svelte (1)
Line range hint
118-165
: LGTM!Navigation structure matches design requirements with clear sections and separators.
path: '/GroupBys', | ||
icon: IconRectangleStack, |
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.
🛠️ Refactor suggestion
Fix path casing inconsistency.
Paths should use lowercase for consistency with other routes.
- path: '/GroupBys',
+ path: '/groupbys',
- path: '/StagingQueries',
+ path: '/staging-queries',
Also applies to: 28-29
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.
LGTM. Thanks @ken-zlai
## Summary This PR updates the left navigation to align with the new structure in the Figma design. The updated order places **Home** at the top, followed by **your dataset entities**, and then **resources** at the bottom. In a future PR, I will add templates for the **Models**, **Groupbys**, and **Staging Queries** pages, including tabs for **Observability** and **Job Tracking**. Currently, these routes will result in a 404 error. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update
Summary
This PR updates the left navigation to align with the new structure in the Figma design. The updated order places Home at the top, followed by your dataset entities, and then resources at the bottom.
In a future PR, I will add templates for the Models, Groupbys, and Staging Queries pages, including tabs for Observability and Job Tracking. Currently, these routes will result in a 404 error.
Checklist