Skip to content
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

Fix issue where by default a long media doesnt take up the full screen #1159

Merged
merged 6 commits into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@
totalNumRows={$firstRow?.data?.total_num_rows}
{mediaFields}
{highlightedFields}
datasetViewHeight={320}
nsthorat marked this conversation as resolved.
Show resolved Hide resolved
/>
{/if}
</div>
Expand Down
31 changes: 19 additions & 12 deletions web/blueprint/src/lib/components/datasetView/ItemMedia.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
type LilacValueNode,
type Path
} from '$lilac';
import {SkeletonText} from 'carbon-components-svelte';
import {
CatalogPublish,
ChevronDown,
Expand All @@ -56,6 +55,7 @@
// The root path contains the sub-path up to the point of this leaf.
export let rootPath: Path | undefined = undefined;
export let isFetching: boolean | undefined = undefined;
export let datasetViewHeight: number | undefined = undefined;

let childPathParts: string[];
// Find all the children path parts that match a media field.
Expand All @@ -74,7 +74,7 @@
if (lastMediaPath == null) continue;

const subPath = [...rootPath, field.path[rootPath.length]];
const valueNodes = getValueNodes(row!, subPath);
const valueNodes = row != null ? getValueNodes(row, subPath) : [];
for (const childNode of valueNodes) {
const childPath = L.path(childNode)![rootPath.length];
if (childPath != null) {
Expand Down Expand Up @@ -115,7 +115,7 @@
}
}

$: valueNodes = row != null ? getValueNodes(row, rootPath!) : [];
$: valueNodes = row != null ? getValueNodes(row, rootPath!) : null;

// The child component will communicate this back upwards to this component.
let textIsOverBudget = false;
Expand All @@ -126,7 +126,7 @@
const datasetViewStore = getDatasetViewContext();
const appSettings = getSettingsContext();

$: value = L.value(valueNodes[0]) as string;
$: value = valueNodes != null ? (L.value(valueNodes[0]) as string) : null;

$: settings = querySettings($datasetViewStore.namespace, $datasetViewStore.datasetName);

Expand Down Expand Up @@ -329,23 +329,21 @@
{/if}

<div class="grow pt-1">
{#if isFetching}
<SkeletonText class="!w-80" />
{:else if value == null || row == null}
<span class="ml-12 italic">null</span>
{:else if colCompareState == null && spanValuePaths != null && field != null}
{#if colCompareState == null && field != null}
<ItemMediaTextContent
hidden={markdown}
text={value}
{row}
path={rootPath}
{field}
isExpanded={userExpanded}
spanPaths={spanValuePaths.spanPaths}
spanValueInfos={spanValuePaths.spanValueInfos}
spanPaths={spanValuePaths?.spanPaths || []}
spanValueInfos={spanValuePaths?.spanValueInfos || []}
{datasetViewStore}
embeddings={computedEmbeddings}
{viewType}
{isFetching}
{datasetViewHeight}
bind:textIsOverBudget
/>
<div class="markdown w-full" class:hidden={!markdown}>
Expand All @@ -354,7 +352,14 @@
</div>
</div>
{:else if colCompareState != null}
<ItemMediaDiff {row} {colCompareState} bind:textIsOverBudget isExpanded={userExpanded} />
<ItemMediaDiff
{row}
{colCompareState}
bind:textIsOverBudget
isExpanded={userExpanded}
{datasetViewHeight}
{isFetching}
/>
{/if}
</div>
</div>
Expand Down Expand Up @@ -387,6 +392,8 @@
{mediaFields}
{row}
{highlightedFields}
{datasetViewHeight}
{isFetching}
/>
</div>
{/each}
Expand Down
38 changes: 27 additions & 11 deletions web/blueprint/src/lib/components/datasetView/ItemMediaDiff.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,29 @@
import type * as Monaco from 'monaco-editor/esm/vs/editor/editor.api';
import {onDestroy, onMount} from 'svelte';

import {getMonaco, MONACO_OPTIONS} from '$lib/monaco';
import {
DEFAULT_HEIGHT_PEEK_SCROLL_PX,
DEFAULT_HEIGHT_PEEK_SINGLE_ITEM_PX,
getMonaco,
MONACO_OPTIONS
} from '$lib/monaco';
import {getDatasetViewContext, type ColumnComparisonState} from '$lib/stores/datasetViewStore';
import {getDisplayPath} from '$lib/view_utils';
import {getValueNodes, L, type LilacValueNode} from '$lilac';
import {getValueNodes, L, type DatasetUISettings, type LilacValueNode} from '$lilac';
import {PropertyRelationship} from 'carbon-icons-svelte';
import {hoverTooltip} from '../common/HoverTooltip';

const MAX_MONACO_HEIGHT_COLLAPSED = 360;
const MAX_MONACO_HEIGHT_EXPANDED = 720;

const datasetViewStore = getDatasetViewContext();

export let row: LilacValueNode;
export let row: LilacValueNode | undefined | null;
export let colCompareState: ColumnComparisonState;
export let textIsOverBudget: boolean;
export let isExpanded: boolean;
export let viewType: DatasetUISettings['view_type'] | undefined = undefined;
export let datasetViewHeight: number | undefined = undefined;
export let isFetching: boolean | undefined = undefined;

let editorContainer: HTMLElement;

Expand All @@ -27,8 +34,8 @@
$: rightPath = colCompareState.swapDirection
? colCompareState.column
: colCompareState.compareToColumn;
$: leftValue = L.value(getValueNodes(row, leftPath)[0]) as string;
$: rightValue = L.value(getValueNodes(row, rightPath)[0]) as string;
$: leftValue = row != null ? (L.value(getValueNodes(row, leftPath)[0]) as string) : '';
$: rightValue = row != null ? (L.value(getValueNodes(row, rightPath)[0]) as string) : '';

let monaco: typeof Monaco;
let editor: Monaco.editor.IStandaloneDiffEditor;
Expand All @@ -38,7 +45,10 @@
relayout();
}
}

$: maxMonacoHeightCollapsed = datasetViewHeight
? datasetViewHeight -
(viewType === 'scroll' ? DEFAULT_HEIGHT_PEEK_SCROLL_PX : DEFAULT_HEIGHT_PEEK_SINGLE_ITEM_PX)
: MAX_MONACO_HEIGHT_COLLAPSED;
function relayout() {
if (
editor != null &&
Expand All @@ -51,10 +61,12 @@
);
textIsOverBudget = contentHeight > MAX_MONACO_HEIGHT_COLLAPSED;

if (isExpanded || !textIsOverBudget) {
editorContainer.style.height = `${Math.min(contentHeight, MAX_MONACO_HEIGHT_EXPANDED)}px`;
if (isExpanded) {
editorContainer.style.height = contentHeight + 'px';
} else if (!textIsOverBudget) {
editorContainer.style.height = `${Math.min(contentHeight, maxMonacoHeightCollapsed)}px`;
} else {
editorContainer.style.height = MAX_MONACO_HEIGHT_COLLAPSED + 'px';
editorContainer.style.height = maxMonacoHeightCollapsed + 'px';
}
editor.layout();
}
Expand Down Expand Up @@ -93,7 +105,8 @@
});
</script>

<div class="relative -ml-6 flex h-fit w-full flex-col gap-x-4">
<!-- For reasons unknown to me, the -ml-6 is required to make the autolayout of monaco react. -->
<div class="relative left-16 -ml-10 flex h-fit w-full flex-col gap-x-4 pr-6">
<div class="flex flex-row items-center font-mono text-xs font-medium text-neutral-500">
<div class="ml-8 w-1/2">{getDisplayPath(leftPath)}</div>
<div class="ml-8 w-1/2">{getDisplayPath(rightPath)}</div>
Expand All @@ -106,6 +119,9 @@
</div>
</div>
<div class="editor-container" bind:this={editorContainer} />
{#if isFetching}
nsthorat marked this conversation as resolved.
Show resolved Hide resolved
<div class="absolute inset-0 flex items-center justify-center bg-white bg-opacity-70" />
{/if}
</div>

<style lang="postcss">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@
import {onDestroy, onMount} from 'svelte';

import {
DEFAULT_HEIGHT_PEEK_SCROLL_PX,
DEFAULT_HEIGHT_PEEK_SINGLE_ITEM_PX,
MAX_MONACO_HEIGHT_COLLAPSED,
MAX_MONACO_HEIGHT_EXPANDED,
MONACO_OPTIONS,
getMonaco
} from '$lib/monaco';
Expand All @@ -35,9 +36,9 @@
import {SkeletonText} from 'carbon-components-svelte';
import {derived} from 'svelte/store';
import {getMonacoRenderSpans, type MonacoRenderSpan, type SpanValueInfo} from './spanHighlight';
export let text: string;
export let text: string | null | undefined;
// The full row item.
export let row: LilacValueNode;
export let row: LilacValueNode | undefined | null;
export let field: LilacField | undefined = undefined;
// Path of the spans for this item to render.
export let spanPaths: Path[];
Expand All @@ -47,13 +48,13 @@
// Information about each value under span paths to render.
export let spanValueInfos: SpanValueInfo[];
export let embeddings: string[];

// When defined, enables semantic search on spans.
export let datasetViewStore: DatasetViewStore | undefined = undefined;
export let isExpanded = false;
// Passed back up to the parent.
export let textIsOverBudget = false;
export let viewType: DatasetUISettings['view_type'] | undefined = undefined;
export let datasetViewHeight: number | undefined = undefined;
export let isFetching: boolean | undefined = undefined;

// Map paths from the searches to the spans that they refer to.
let pathToSpans: {
Expand All @@ -62,6 +63,7 @@
$: {
pathToSpans = {};
spanPaths.forEach(sp => {
if (row == null) return;
let valueNodes = getValueNodes(row, sp);
const isSpanNestedUnder = pathMatchesPrefix(sp, path);
if (isSpanNestedUnder) {
Expand Down Expand Up @@ -114,24 +116,32 @@
}
}

$: maxMonacoHeightCollapsed = datasetViewHeight
? datasetViewHeight -
(viewType === 'scroll' ? DEFAULT_HEIGHT_PEEK_SCROLL_PX : DEFAULT_HEIGHT_PEEK_SINGLE_ITEM_PX)
: MAX_MONACO_HEIGHT_COLLAPSED;

function relayout() {
if (editor != null && editor.getModel() != null) {
if (editorContainer != null && editor != null && editor.getModel() != null) {
const contentHeight = editor.getContentHeight();
textIsOverBudget = contentHeight > MAX_MONACO_HEIGHT_COLLAPSED;
textIsOverBudget = contentHeight > maxMonacoHeightCollapsed;

if (isExpanded || !textIsOverBudget) {
editorContainer.style.height = `${Math.min(contentHeight, MAX_MONACO_HEIGHT_EXPANDED)}px`;
if (isExpanded) {
editorContainer.style.height = contentHeight + 'px';
} else if (!textIsOverBudget) {
editorContainer.style.height = `${Math.min(contentHeight, maxMonacoHeightCollapsed)}px`;
} else {
editorContainer.style.height = MAX_MONACO_HEIGHT_COLLAPSED + 'px';
editorContainer.style.height = maxMonacoHeightCollapsed + 'px';
}

editor.layout();
}
}

onMount(async () => {
if (editorContainer == null) return;
if (monaco != null) return;
monaco = await getMonaco();
if (editorContainer == null) return;
editor = monaco.editor.create(editorContainer, {
...MONACO_OPTIONS,
lineNumbers: 'on',
Expand All @@ -145,7 +155,7 @@
scale: 2
}
});
model = monaco.editor.createModel(text, 'text/plain');
model = monaco.editor.createModel(text || '', 'text/plain');
editor.setModel(model);

// When the fonts are ready, measure the fonts and display the editor after the font measurement
Expand All @@ -159,13 +169,13 @@

// When text changes, set the value on the global model and relayout.
$: {
if (model != null && text != null) {
if (editorContainer != null && model != null && text != null) {
model.setValue(text);
relayout();
}
}

$: monacoSpans = getMonacoRenderSpans(text, pathToSpans, spanPathToValueInfos);
$: monacoSpans = getMonacoRenderSpans(text || '', pathToSpans, spanPathToValueInfos);

// Reveal the first span so that it is near the top of the view.
$: {
Expand Down Expand Up @@ -653,6 +663,9 @@
bind:this={editorContainer}
class:invisible={!editorReady}
/>
{#if isFetching}
<div class="absolute inset-0 flex items-center justify-center bg-white bg-opacity-70" />
{/if}
</div>

<style lang="postcss">
Expand Down
26 changes: 18 additions & 8 deletions web/blueprint/src/lib/components/datasetView/RowItem.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
undefined;
export let nextRowId: string | undefined = undefined;
export let modalOpen = false;
export let datasetViewHeight: number;

let openDeleteModal = false;

Expand Down Expand Up @@ -80,14 +81,14 @@
$selectRowsSchema.data?.schema,
rowId != null && $selectRowsSchema.data != null /* enabled */
);
$: row = $rowQuery != null && !$rowQuery.isFetching ? $rowQuery?.data : null;
$: row = $rowQuery.data;
$: rowLabels = row != null ? getRowLabels(row) : [];
$: disableLabels = !canEditLabels;

$: schemaLabels = $schema.data && getSchemaLabels($schema.data);
$: addLabels = $schema.data != null ? addLabelsMutation($schema.data) : null;

$: isStale = $rowQuery?.isStale;
$: isStale = $rowQuery.isStale;
$: {
if (!isStale) {
labelsInProgress = new Set();
Expand Down Expand Up @@ -170,6 +171,8 @@
}
}
}

let headerHeight: number;
</script>

<div class="relative flex w-full flex-col rounded md:flex-col">
Expand All @@ -181,6 +184,7 @@
class:bg-opacity-70={!$datasetViewStore.viewTrash}
class:bg-red-500={$datasetViewStore.viewTrash}
class:bg-opacity-20={$datasetViewStore.viewTrash}
bind:clientHeight={headerHeight}
>
<!-- Left arrow -->
<div class="flex w-1/3 flex-row">
Expand Down Expand Up @@ -300,14 +304,20 @@
bind:clientHeight={mediaHeight}
>
<div class="rounded-b border-b border-l border-r border-neutral-300">
{#if $rowQuery?.isFetching}
<SkeletonText class="w-20" />
{/if}
{#if mediaFields.length > 0 && row != null}
<div class="flex w-full flex-col">
<ItemMedia {mediaFields} {row} {highlightedFields} isFetching={$rowQuery?.isFetching} />
{#if $rowQuery.isFetching}
<div class="absolute top-0 h-2 w-full overflow-hidden">
<SkeletonText class="w-20" lines={1} />
</div>
{/if}
<div class="flex w-full flex-col">
<ItemMedia
{mediaFields}
{row}
{highlightedFields}
isFetching={$rowQuery.isFetching}
datasetViewHeight={datasetViewHeight - headerHeight}
/>
</div>
</div>
</div>
{#if $datasetViewStore.showMetadataPanel}
Expand Down
Loading
Loading