Skip to content

Fix: improve root directory modal behavior and UX #2453

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.

Sign up for GitHub

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

Merged
HarshMN2345 merged 13 commits into from
Feb 20, 2026
Merged

Fix: improve root directory modal behavior and UX #2453

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
c3fb402
fix: Buggy root directory Modal
HarshMN2345 Oct 6, 2025
45e9a40
some fixes
HarshMN2345 Oct 6, 2025
5c31ce2
Merge remote-tracking branch 'origin/main' into fix-SER-428-Quality-C…
HarshMN2345 Feb 16, 2026
16f9f46
fix: lag
HarshMN2345 Feb 16, 2026
0181088
svelte icon fix
HarshMN2345 Feb 16, 2026
7f31579
some more chnages
HarshMN2345 Feb 19, 2026
aecdfd5
svelte 5
HarshMN2345 Feb 19, 2026
c849eed
Merge remote-tracking branch 'origin/main' into fix-SER-428-Quality-C…
HarshMN2345 Feb 19, 2026
9f13711
addressed code rabbit comments
HarshMN2345 Feb 19, 2026
523f228
Merge remote-tracking branch 'origin/main' into fix-SER-428-Quality-C…
HarshMN2345 Feb 20, 2026
affe37f
use onresize
HarshMN2345 Feb 20, 2026
0079071
fix onSelect and onChange are both wired to handleSelect, causing it …
HarshMN2345 Feb 20, 2026
5d4a44a
fix domains table lint
HarshMN2345 Feb 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
247 changes: 247 additions & 0 deletions src/lib/components/git/DirectoryItem.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,247 @@
<script lang="ts">
import { getContext } from 'svelte';
import type { createTreeView } from '@melt-ui/svelte';
import { IconChevronRight } from '@appwrite.io/pink-icons-svelte';
import { Icon, Layout, Selector, Spinner, Typography } from '@appwrite.io/pink-svelte';
import DirectoryItemSelf from './DirectoryItem.svelte';
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

The self-import on line 6 uses the alias DirectoryItemSelf which is the same component importing itself. While this is technically correct for recursive components in Svelte 5, the name DirectoryItemSelf is not clear about its purpose. Consider using a more descriptive name like DirectoryItemRecursive or just referencing the component by its original name if that's clearer in context.

Copilot uses AI. Check for mistakes.

let {
directories,
level = 0,
containerWidth,
selectedPath,
onSelect
}: {
directories: Array<{
title: string;
fileCount?: number;
fullPath: string;
thumbnailUrl?: string;
thumbnailIcon?: typeof Icon;
thumbnailHtml?: string;
children?: typeof directories;
hasChildren?: boolean;
showThumbnail?: boolean;
loading?: boolean;
}>;
level?: number;
containerWidth?: number;
selectedPath?: string;
onSelect?: (detail: { title: string; fullPath: string; hasChildren: boolean }) => void;
} = $props();

const Radio = Selector.Radio;

let radioInputs = $state<Array<HTMLInputElement | undefined>>([]);
let value = $state<string | undefined>(undefined);
let thumbnailStates = $state<Array<{ loading: boolean; error: boolean }>>([]);

$effect(() => {
if (!directories) return;
if (thumbnailStates.length < directories.length) {
thumbnailStates = [
...thumbnailStates,
...Array.from({ length: directories.length - thumbnailStates.length }, () => ({
loading: true,
error: false
}))
];
} else if (thumbnailStates.length > directories.length) {
thumbnailStates = thumbnailStates.slice(0, directories.length);
}
});

function handleThumbnailLoad(index: number) {
if (!thumbnailStates[index]) return;
thumbnailStates[index].loading = false;
thumbnailStates[index].error = false;
}

function handleThumbnailError(index: number) {
if (!thumbnailStates[index]) return;
thumbnailStates[index].loading = false;
thumbnailStates[index].error = true;
Comment on lines +55 to +63
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

The mutation of thumbnailStates array elements on lines 56-57 and 62-63 may not trigger reactivity in Svelte 5. Consider replacing the entire array element instead of mutating properties. For example, use: thumbnailStates[index] = { ...thumbnailStates[index], loading: false, error: false }; or use thumbnailStates = thumbnailStates.with(index, { loading: false, error: false }) to ensure reactivity.

Suggested change
if (!thumbnailStates[index]) return;
thumbnailStates[index].loading = false;
thumbnailStates[index].error = false;
}
function handleThumbnailError(index: number) {
if (!thumbnailStates[index]) return;
thumbnailStates[index].loading = false;
thumbnailStates[index].error = true;
const current = thumbnailStates[index];
if (!current) return;
thumbnailStates = thumbnailStates.with(index, {
...current,
loading: false,
error: false
});
}
function handleThumbnailError(index: number) {
const current = thumbnailStates[index];
if (!current) return;
thumbnailStates = thumbnailStates.with(index, {
...current,
loading: false,
error: true
});

Copilot uses AI. Check for mistakes.
}

const {
elements: { item, group },
helpers: { isExpanded }
} = getContext<ReturnType<typeof createTreeView>>('tree');

const paddingLeftStyle = `padding-left: ${32 * level + 8}px`;

$effect(() => {
if (selectedPath && directories?.length) {
const idx = directories.findIndex((d) => d.fullPath === selectedPath);
if (idx !== -1 && radioInputs[idx]) {
radioInputs[idx].checked = true;
Comment on lines +76 to +77
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

Directly setting radioInputs[idx].checked = true mutates a DOM element property, which may not properly sync with Svelte's reactivity system. Consider using a controlled approach by managing the checked state through Svelte state and binding, or ensuring that the Radio component's value is properly synchronized instead of directly manipulating the DOM.

Suggested change
if (idx !== -1 && radioInputs[idx]) {
radioInputs[idx].checked = true;
if (idx !== -1) {
value = directories[idx].fullPath;

Copilot uses AI. Check for mistakes.
}
}
});
</script>

{#each directories as { title, fileCount, fullPath, thumbnailUrl, thumbnailIcon, thumbnailHtml, children, hasChildren: explicitHasChildren, showThumbnail = true, loading = false }, i}
{@const hasChildren = explicitHasChildren ?? !!children?.length}
{@const __MELTUI_BUILDER_1__ = $group({ id: fullPath })}
{@const __MELTUI_BUILDER_0__ = $item({
id: fullPath,
hasChildren
})}

<div class="directory-item-container">
<button
class="folder"
type="button"
style={paddingLeftStyle}
onclick={() => {
if (radioInputs[i]) radioInputs[i].checked = true;
onSelect?.({ title, fullPath, hasChildren });
}}
{...__MELTUI_BUILDER_0__}
use:__MELTUI_BUILDER_0__.action>
Comment on lines +92 to +101
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

The button element on lines 92-166 lacks an accessible label (aria-label) that describes its purpose. While it contains visual elements like title and icons, screen reader users may not get a clear indication of what the button does. Consider adding an aria-label that includes the directory name and its state (e.g., aria-label="Select directory: {title}, {hasChildren ? 'expandable' : 'not expandable'}")

Copilot uses AI. Check for mistakes.
<Layout.Stack direction="row" justifyContent="space-between">
<Layout.Stack
direction="row"
justifyContent="flex-start"
gap="xxs"
alignItems="center">
<div>
<Layout.Stack direction="row" gap="xxs" alignItems="center">
<Radio
group="directory"
name="directory"
size="s"
bind:value
bind:radioInput={radioInputs[i]} />
<div
class:folder-open={$isExpanded(fullPath)}
class:disabled={!hasChildren}
class="chevron-container">
<Icon
icon={IconChevronRight}
size="s"
color="--fgcolor-neutral-tertiary" />
</div>
</Layout.Stack>
</div>
<span
class="title"
style={containerWidth
? `max-width: ${containerWidth - 100 - level * 40}px`
: ''}>{title}</span>
{#if fileCount !== undefined}
<div class="fileCount">
<Typography.Text variant="m-400" color="--fgcolor-neutral-tertiary"
>({fileCount} files)</Typography.Text>
</div>
{/if}
</Layout.Stack>
{#if showThumbnail}
{#if loading || (thumbnailStates[i]?.loading && !thumbnailIcon && !thumbnailHtml)}
<Spinner />
{/if}

{#if thumbnailStates[i]?.error}
<div class="thumbnail-fallback"></div>
{:else if thumbnailUrl}
<img
src={thumbnailUrl}
alt="Directory thumbnail"
class="thumbnail"
class:hidden={thumbnailStates[i]?.loading}
onload={() => handleThumbnailLoad(i)}
onerror={() => handleThumbnailError(i)} />
{:else if thumbnailIcon}
<div class="thumbnail">
<Icon icon={thumbnailIcon} size="l" />
</div>
{:else if thumbnailHtml}
<div class="thumbnail">
<!-- eslint-disable-next-line svelte/no-at-html-tags -->
{@html thumbnailHtml}
</div>
{/if}
{/if}
</Layout.Stack>
</button>

{#if children}
<div {...__MELTUI_BUILDER_1__} use:__MELTUI_BUILDER_1__.action>
<DirectoryItemSelf
directories={children}
level={level + 1}
{containerWidth}
{selectedPath}
{onSelect} />
</div>
{/if}
</div>
{/each}

<style>
.directory-item-container {
width: 100%;
}
.folder {
display: flex;
width: 100%;
flex-direction: row;
padding: var(--space-3, 6px) var(--space-4, 8px);
justify-content: space-between;
align-items: center;
cursor: pointer;
-webkit-tap-highlight-color: rgba(0, 0, 0, 0);

&:hover,
&:focus {
border-radius: var(--border-radius-s, 8px);
background: var(--bgcolor-neutral-secondary, #f4f4f7);
}
}
.chevron-container {
width: var(--space-7);
height: var(--space-7);
transition: transform ease-in-out 0.1s;
}
.folder-open {
transform: rotate(90deg);
}
.disabled {
color: var(--fgcolor-neutral-tertiary);
}

.title {
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
flex-grow: 0;
}

.fileCount {
display: none;

@media (min-width: 1024px) {
display: block;
}
}

.hidden {
display: none;
}

.thumbnail {
width: var(--icon-size-l, 24px);
height: var(--icon-size-l, 24px);
flex-shrink: 0;
border-radius: var(--border-radius-circle, 99999px);
}

.thumbnail-fallback {
width: var(--icon-size-l, 24px);
height: var(--icon-size-l, 24px);
flex-shrink: 0;
border-radius: var(--border-radius-circle, 99999px);
border: var(--border-width-s, 1px) dashed var(--border-neutral-strong, #d8d8db);
background: var(--bgcolor-neutral-primary, #fff);
}
</style>
132 changes: 132 additions & 0 deletions src/lib/components/git/DirectoryPicker.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
<script lang="ts">
import { createTreeView } from '@melt-ui/svelte';
import { onMount, setContext } from 'svelte';
import { writable, type Writable } from 'svelte/store';
import DirectoryItem from '$lib/components/git/DirectoryItem.svelte';
import type { DirectoryEntry } from '$lib/components/git/types';
import { Spinner } from '@appwrite.io/pink-svelte';

let {
expanded = $bindable(writable(['lib-0', 'tree-0'])),
selected = $bindable(undefined),
openTo,
directories,
isLoading = true,
onSelect,
onChange
}: {
expanded?: Writable<string[]>;
selected?: string;
openTo?: string;
directories: DirectoryEntry[];
isLoading?: boolean;
onSelect?: (detail: {
fullPath: string;
hasChildren: boolean;
title: string;
}) => void | Promise<void>;
onChange?: (detail: { fullPath: string }) => void | Promise<void>;
} = $props();

const ctx = createTreeView({ expanded });
setContext('tree', ctx);

const {
elements: { tree }
} = ctx;

let rootContainer = $state<HTMLDivElement | undefined>(undefined);
let containerWidth = $state<number | undefined>(undefined);
let internalSelected = $state<string | undefined>(undefined);

$effect(() => {
internalSelected = selected;
});

onMount(() => {
updateWidth();
if (openTo) {
const pathSegments = openTo.split('/').filter(Boolean);
const pathsToExpand: string[] = [];
let currentPath = '';
for (const segment of pathSegments) {
currentPath += '/' + segment;
pathsToExpand.push(currentPath);
}
if (pathsToExpand.length > 0) {
expanded?.update((current) => {
const next = [...current];
pathsToExpand.forEach((path) => {
if (!next.includes(path)) {
next.push(path);
}
});
return next;
});
}
}
});
Comment on lines +46 to +68
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

The logic on lines 46-68 in onMount expands paths based on the openTo prop. However, this logic doesn't account for the actual directory structure—it blindly adds paths without checking if those directories exist. This could lead to inconsistencies or unexpected behavior. Consider coordinating this with the actual directory loading logic in the parent component or deferring expansion until directories are loaded.

Copilot uses AI. Check for mistakes.

function updateWidth() {
containerWidth = rootContainer ? rootContainer.getBoundingClientRect().width : undefined;
}

function handleSelect(detail: { fullPath: string; hasChildren: boolean; title: string }) {
internalSelected = detail.fullPath;
selected = internalSelected;
if (onChange) onChange({ fullPath: detail.fullPath });
if (onSelect) onSelect(detail);
}

$effect(() => {
containerWidth = rootContainer ? rootContainer.getBoundingClientRect().width : undefined;
});
Comment on lines +80 to +83
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

The effect on lines 81-83 recalculates containerWidth every time the effect runs, but this will be triggered by any reactive dependency change including rootContainer. This creates unnecessary recalculations. Consider using a more specific dependency or removing this effect since the width is already updated on mount and window resize.

Suggested change
$effect(() => {
containerWidth = rootContainer ? rootContainer.getBoundingClientRect().width : undefined;
});

Copilot uses AI. Check for mistakes.
</script>

<svelte:window onresize={updateWidth} />

<div class="directory-container" class:isLoading {...$tree} bind:this={rootContainer}>
{#if isLoading}
<div class="loading-container">
<Spinner /><span>Loading directory data...</span>
</div>
{:else}
<DirectoryItem
{directories}
{containerWidth}
selectedPath={internalSelected}
onSelect={handleSelect} />
{/if}
</div>

<style>
.directory-container {
width: 560px;
max-width: 100%;
height: 316px;
overflow-y: auto;
flex-shrink: 0;
display: flex;
padding: var(--space-2, 4px);

border-radius: var(--border-radius-m, 12px);
border: var(--border-width-s, 1px) solid var(--border-neutral, #ededf0);
background: var(--bgcolor-neutral-primary, #fff);

&::-webkit-scrollbar {
display: none;
}
}

.isLoading {
justify-content: center;
align-items: center;
}

.loading-container {
display: flex;
flex-direction: column;
align-items: center;
gap: var(--gap-m);
}
</style>
Loading