Feature/show all columns at once#782
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the file list to show all available columns by default and shifts column sizing from percent-based widths to pixel-based widths, adding supporting UX for reordering and more efficient horizontal rendering.
Changes:
- Converted persisted/runtime column widths from percent (0–1) to pixel units across core state, UI components, URL encoding/decoding, and tests.
- Added a
reorderColumnsaction/reducer path and updated header drag/drop and context menu interactions to support moving columns to start/end. - Implemented horizontal “cell virtualization” via a scroll context +
useVisibleCellshook so only visible columns render per row.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/desktop/src/services/test/PersistentConfigServiceElectron.test.ts | Updates persisted column width test fixtures to pixel units. |
| packages/core/state/selection/test/reducer.test.ts | Updates width fixtures to px and adds reducer tests for REORDER_COLUMNS. |
| packages/core/state/selection/selectors.ts | Replaces percent overflow selector with getTotalColumnWidth selector. |
| packages/core/state/selection/reducer.ts | Removes RESIZE_COLUMN handling and adds REORDER_COLUMNS reducer logic. |
| packages/core/state/selection/logics.ts | Adds RESIZE_COLUMN logic to translate resize intent into setColumns updates. |
| packages/core/state/selection/actions.ts | Updates column/resize action typing for px widths; adds REORDER_COLUMNS action. |
| packages/core/state/metadata/test/logics.test.ts | Updates width fixtures; adds tests ensuring annotations default into columns. |
| packages/core/state/metadata/logics.ts | Changes receive-annotations behavior to add all annotations as columns with default px widths. |
| packages/core/entity/SearchParams/index.ts | Introduces DEFAULT_COLUMN_WIDTH, updates default query widths, and changes column URL encode/decode to px-based scheme. |
| packages/core/components/FileRow/index.tsx | Adds left/right padding support to accommodate horizontally virtualized column rendering. |
| packages/core/components/FileRow/Cell.tsx | Converts cell sizing/resizing from percent widths to pixel widths. |
| packages/core/components/FileRow/Cell.module.css | Refactors resize-handle spacing via CSS variables. |
| packages/core/components/FileList/useVisibleCells.ts | Adds hook to compute visible column slice + spacer padding from horizontal scroll state. |
| packages/core/components/FileList/useDragAndDropOrder.ts | Changes reorder callback signature to (item, moveTo) instead of passing full new order. |
| packages/core/components/FileList/test/useVisibleColumns.test.tsx | Adds test coverage for the visible-columns hook behavior. |
| packages/core/components/FileList/test/useDragAndDropOrder.test.tsx | Updates tests for new drag/drop reorder callback signature. |
| packages/core/components/FileList/test/LazilyRenderedRow.test.tsx | Updates width fixtures to px. |
| packages/core/components/FileList/test/Header.test.tsx | Updates header drag/drop test to assert reorderColumns dispatch. |
| packages/core/components/FileList/LazilyRenderedRow.tsx | Renders only visible columns and sets row width from total column width in px. |
| packages/core/components/FileList/index.tsx | Provides horizontal scroll context and scroll tracking; memoizes list itemData. |
| packages/core/components/FileList/HorizontalScrollContext.ts | Adds context for scrollLeft + containerWidth. |
| packages/core/components/FileList/Header.tsx | Removes column picker context menu; adds move-to-start/end context menu; uses visible-columns hook. |
| packages/core/components/FileList/Header.module.css | Adjusts header layout to support truncation/ellipsis with new click targets. |
| packages/core/components/FileList/ColumnPicker.tsx | Updates default added-column width to px. |
| packages/core/components/DirectoryTree/test/DirectoryTree.test.tsx | Updates width fixtures to px. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
BrianWhitneyAI
left a comment
There was a problem hiding this comment.
So if I am understanding correctly, This removes any column picking in favor of ordering the columns you want to the front. Is that what we want to do rather than just switch to "all columns" option for users? Should we be considering a way to hide columns?
| el.scrollLeft = target; | ||
| } | ||
| }, [fileSet.instanceId, fileView]); | ||
|
|
There was a problem hiding this comment.
This restores scroll position after the InfiniteLoader remounts (e.g. after a sort), but it fires on any fileSet.instanceId change — including when the user switches to a completely different dataset. If the user was scrolled to 500px in dataset A and then opens dataset B, this will scroll B to 500px immediately. Consider resetting lastScrollLeftRef.current to 0 when the instanceId changes to distinguish "remount due to sort" from "switched to new dataset."
There was a problem hiding this comment.
I may be misunderstanding, but I don't think I've been able to replicate this comment
| // Re-attach listener when totalCount changes, as react-window may remount the outerRef div in that case | ||
| // but also when the fileView changes so that the scroll position can be reset | ||
| // and when measuredWidth changes to ensure containerWidth is accurate for useVisibleCells (like if window size changes) | ||
| }, [fileSet.instanceId, fileView, measuredWidth, totalCount]); |
There was a problem hiding this comment.
Per Claude Code - The totalCount dep here looks incorrect. The comment says react-window may remount outerRef when totalCount changes, but FixedSizeList is keyed on fileSet.instanceId and fileView — it remounts on those, not on count changes. Including totalCount means the scroll listener is torn down and containerWidth is re-initialized on every page of data that loads in, which briefly collapses the rendered columns to the first-6 fallback. I think you can drop totalCount from this dep array.
| { name: AnnotationName.FILE_NAME, width: 300 }, | ||
| { name: AnnotationName.KIND, width: 150 }, | ||
| { name: AnnotationName.TYPE, width: 200 }, | ||
| { name: AnnotationName.FILE_SIZE, width: 100 }, |
There was a problem hiding this comment.
I might be missing something here but whats the reasoning for switching from percentages/ proportions to pixels?
There was a problem hiding this comment.
Couple reasons:
a) Proportions got funky when switching between users with different window sizes
b) Proportions relative to an essentially infinite list gets tricky and relies on reaching outside of the component and into the parent to measure the width which felt icky - though it is possible!
|
I'm conflicted about fully removing the columns menu, it seems like it could be frustrating to only be able to sort/manage columns this way. In particular, if you have hundreds of columns (e.g., the NucMorph dataset, which has 658), you now have to scroll through to find the ones you want since there's no longer a search function. I can also imagine that users who are using this for publishing data might want to have the ability to just show a subset of columns? |
Re: searching for columnsI hadn't really considered this! Nobody seemed to bring it up in my demos, but this is a great point. I'm not sure what is the best thing to do here to solve this, the existing "Modify columns" action + column picker UI has never not been confusing to users I've seen using BFF so I'm not sure that is the answer either. Re: hiding columnsI wonder about whether users will want to hide columns as well. None of the datasets that have been published alongside BFF seems to have too many columns & I talked with Antoine about this for the EMT dataset and he wasn't concerned - but it does seem potentially tricky. The reason I didn't include some way to only show a subset in this is because I think showing all columns is better than showing a small random subset by default which seem to be our only two options. Otherwise we get into issues with trying to store too much information in the query arguments, like for example:
I can't think of any optimizations we could do that would get better storage performance than up to X / 2 columns in the query args in the worst case (X = number of columns). My thought was try publishing this and then if we want to add on the ability to hide columns we could add it in. FWIW the way I imagine we could go about storing columns as query args is:
Let me know your thoughts! (fwiw NucMorph is a draft of a draft of a draft of a dataset where most of that metadata is irrelevant, but I totally see your point and is is very true/fair) |
|
re: searching columns re: hiding columns I'm on board with moving forward with this as is for now and thinking about hiding columns as a separate issue. Will finish reviewing! |
| const onHeaderNameClick = (evt: React.MouseEvent, columnName: string) => { | ||
| // Prevent this click from bubbling up to the header's onClick | ||
| // which opens the column picker context menu | ||
| evt.stopPropagation(); | ||
| dispatch(selection.actions.sortColumn(column.name)); | ||
| dispatch(selection.actions.sortColumn(columnName)); | ||
| }; | ||
|
|
||
| const headerCells: CellConfig[] = map(columns, (column) => ({ | ||
| const onHeaderColumnClick = (evt: React.MouseEvent, columnName: string) => { |
There was a problem hiding this comment.
What do you think of combining these so that sort is an option in the menu? When the columns are more narrow it's hard to click the header without widening the column, and I'm not sure if users would know to distinguish between those click targets. I know they were separate before bc one was acting on the column itself and the other was acting on the header as a whole, but now the menu is also column-specific
Screen.Recording.2026-05-19.at.1.12.37.PM.mov
There was a problem hiding this comment.
I think that makes sense! I'll run it by UX and report back if they disagree, otherwise will implement
| private static readonly COLUMN_DELIMETER = ","; | ||
| private static readonly VALUE_DELIMETER = ":"; |
There was a problem hiding this comment.
technically unrelated to this PR, but minor typo in these names, should be COLUMN_DELIMITER and VALUE_DELIMITER as opposed to delimeter
There was a problem hiding this comment.
classic - i can update this
| el.scrollLeft = target; | ||
| } | ||
| }, [fileSet.instanceId, fileView]); | ||
|
|
There was a problem hiding this comment.
I may be misunderstanding, but I don't think I've been able to replicate this comment
Context
For, pretty much ever, users have wanted to see all the columns in their dataset at once.
Description
This, by default, adds all columns to the users view. As a result, we can remove the context menu option for modifying the columns. In turn, I've added the option to send the column to the start or end for ease which on top of the previous PR for drag and drop reordering columns should help users create datasets nicely.
A big part of this changeset is converting the width tracking of columns from % of total width to px.
Testing
Tons of local testing. Making sure the column names get padded and cut off properly is truly a nightmare but seems to be good now.
Follow up PRs