Skip to content

Conversation

@KATIETOLER
Copy link
Contributor

@KATIETOLER KATIETOLER commented Jan 14, 2026

Nav Container Update

This PR is meant to add the underlying structure for the new nav.


Add new navigation icons

Adds buildings and cube icons to the Holocene icon library for use in
navigation updates.

Changes

  • New icons: Added buildings.svelte and cube.svelte icon components
  • Icon registry: Updated paths.ts to register new icons
  • Cleanup: Removed unused identities-icon.svelte
  • Navigation: Enhanced navigation container to support both Cloud (with
    subtitle/push icon) and Self-Hosted (with chevron icon) layouts
  • Svelte 5 migration: Converted navigation components from slot syntax to
    snippet syntax for compatibility

Technical Notes

  • Maintains backward compatibility for UI/Core
  • Conditional rendering based on isCloud and subtitle props
  • Supports future subtitle functionality for Cloud repo when packaged

KATIETOLER and others added 5 commits January 9, 2026 14:16
- Add buildings.svelte icon for organizations/projects
- Add cube.svelte icon for deployments/packages
- Remove unused identities-icon.svelte
- Update icon registry in paths.ts

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
- Cloud with subtitle: uses push icon and new layout
- Self-Hosted / Cloud without subtitle: uses chevron icon and standard layout
- Maintains backward compatibility for UI/Core
- Supports new subtitle functionality for Cloud repo

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
- Convert from slot syntax to Svelte 5 snippet syntax
- Update Navigation to use children and bottom snippets
- Update SideNavigation to pass snippets instead of slots
- Update layout to use snippet syntax for bottom nav item

Fixes Vite/Svelte compilation error about mixing slot and render tags

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@vercel
Copy link

vercel bot commented Jan 14, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
holocene Ready Ready Preview, Comment Jan 16, 2026 3:54pm

Request Review

@KATIETOLER KATIETOLER removed the request for review from rossedfort January 14, 2026 23:27
Addresses code review feedback to separate navigation logic into
distinct components instead of using conditional rendering.

- Create CloudNavBar component for Cloud nav with subtitle/push icon
- Create OSSNavBar component for Self-Hosted nav with chevron icon
- Update navigation-container to delegate to appropriate component
- Improves code maintainability and component reusability

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Copy link
Contributor

@andrewzamojc andrewzamojc left a comment

Choose a reason for hiding this comment

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

Looks pretty good. I left a couple comments.

{#if item.divider}
<hr class="-mx-4 my-4 border-subtle" />
<Navigation {isCloud} {bottom} aria-label={translate('common.primary')}>
{#snippet children()}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Children snippet is implicit, so you don't need this line {#snippet children()}. https://svelte.dev/docs/svelte/snippet#Passing-snippets-to-components-Implicit-children-snippet

class="self-center justify-self-center py-3 text-center text-[0.6rem] text-slate-300"
>
<span class="sr-only">{translate('common.version')}</span>
{#if isCloud && subtitle}
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: Should subtitle be part of this if check? If isCloud is true, but subtitle happens to be blank, shouldn't it still render the CloudNavBar just with an blank subtitle='' ?

@temporal-cicd
Copy link
Contributor

temporal-cicd bot commented Jan 15, 2026

Warnings
⚠️

📊 Strict Mode: 2 errors in 2 files (0.2% of 1120 total)

src/lib/holocene/navigation/navigation-container.svelte (1)
  • L33:3: Argument of type '{ accesskey?: string | undefined | null; autocapitalize?: "characters" | "off" | "on" | "none" | "sentences" | "words" | undefined | null; autofocus?: boolean | undefined | null; ... 432 more ...; xmlns?: string | undefined | null; }' is not assignable to parameter of type 'HTMLProps<"nav", HTMLAttributes>'.
src/routes/(app)/+layout.svelte (1)
  • L58:23: Argument of type '(namespace: string) => { namespace: string; onClick: (namespace: string) => void; }' is not assignable to parameter of type '(value: string | null | undefined, index: number, array: (string | null | undefined)[]) => { namespace: string; onClick: (namespace: string) => void; }'.

Generated by 🚫 dangerJS against f623017

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.

3 participants