Skip to content

Optimize column width based on text length#784

Open
SeanDuHare wants to merge 6 commits into
feature/show-all-columns-at-oncefrom
feature/optimize-column-width
Open

Optimize column width based on text length#784
SeanDuHare wants to merge 6 commits into
feature/show-all-columns-at-oncefrom
feature/optimize-column-width

Conversation

@SeanDuHare
Copy link
Copy Markdown
Contributor

Context

Currently the column widths just get set to some arbitrary default. However, a common convention for spreadsheets it to auto-size the column width based on the size of the largest value.

Description

Auto-sizes column widths by default. Width is determined by a fairly accurate estimate using a sample character and the character length of the longest value in the column or the size of the header whichever is larger. Caps the max size of the width for auto-sizing this way by default, but the user can override that.

Testing

Tested locally & added some unit tests

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces default auto-sizing for metadata table columns by estimating an “optimal” pixel width from the longest value length (or header length), instead of relying on a fixed default width.

Changes:

  • Adds AnnotationService.fetchOptimalWidthForAnnotations() and implements it for DB-backed annotations (with a default/fallback implementation for HTTP-backed annotations).
  • Updates selection/metadata redux logics to use auto-sized widths when creating/resetting/resizing columns.
  • Refactors database query typing by introducing a shared DatabaseQuery<T> interface and generic query methods.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
packages/web/src/services/DatabaseServiceWeb/index.ts Updates query() typing to return the new DatabaseQuery<T> shape (currently with an incorrect default generic).
packages/web/src/services/DatabaseServiceWeb/duckdb-worker.worker.ts Updates worker-side query() typing to return DatabaseQuery<T> (currently with an incorrect default generic).
packages/core/state/selection/test/logics.test.ts Loosens mock DB query() typing for updated DatabaseQuery/generic query changes.
packages/core/state/selection/logics.ts Uses fetchOptimalWidthForAnnotations() to auto-size on “reset to default” column resize actions.
packages/core/state/metadata/logics.ts Uses fetchOptimalWidthForAnnotations() to assign widths when new annotations are received.
packages/core/services/FileService/DatabaseFileService/test/DatabaseFileService.test.ts Updates mock DB query() typings to accommodate new query typing.
packages/core/services/DatabaseService/index.ts Introduces DatabaseQuery<T> and makes query()/runQuery() generic.
packages/core/services/DatabaseService/DatabaseServiceNoop.ts Updates noop DB service to return DatabaseQuery<T>.
packages/core/services/AnnotationService/index.ts Adds fetchOptimalWidthForAnnotations() to the AnnotationService interface.
packages/core/services/AnnotationService/HttpAnnotationService/index.ts Adds a placeholder fetchOptimalWidthForAnnotations() implementation returning default widths.
packages/core/services/AnnotationService/DatabaseAnnotationService/test/DatabaseAnnotationService.test.ts Adds unit tests for width computation/capping/fallback behaviors.
packages/core/services/AnnotationService/DatabaseAnnotationService/index.ts Implements width estimation by querying max value lengths and converting to pixel widths using a measured sample character width.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/core/state/metadata/logics.ts
Comment thread packages/web/src/services/DatabaseServiceWeb/index.ts Outdated
Comment thread packages/web/src/services/DatabaseServiceWeb/duckdb-worker.worker.ts Outdated
@SeanDuHare SeanDuHare marked this pull request as ready for review May 11, 2026 19:47
@aswallace
Copy link
Copy Markdown
Contributor

This may just be a weird edge case, but I tested loading this link to the cardio dataset and the column values aren't lining up with the column headers
image

@SeanDuHare
Copy link
Copy Markdown
Contributor Author

this link to the cardio dataset

looks like this is an issue because of file name being a special key column - will fix

Comment on lines +197 to +200
/**
* Fetch the length of the longest value for each annotation, which can be used to compute optimal column widths in the UI.
* This is a bit of a hack, but it allows us to avoid fetching all values for an annotation just to compute column widths.
*/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this might be an accidental copy of the comment for the function below

if (!widthByAnnotation.hasOwnProperty(annotationName)) {
widthByAnnotation[annotationName] = DEFAULT_COLUMN_WIDTH;
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

for the DatabaseService we set optimal widths for TOP_LEVEL_FILE_ANNOTATIONS as a fallback but we don't here, I forget if those get covered by annotationNames in this case?

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.

4 participants