Render only the visible variables in the variables list, and make its rows denser - #9006
Merged
Conversation
The whole tree of variables is flattened into a list of fixed height rows, and only the rows that are visible on screen are rendered. The scroll can be handled by the variables list itself (in the variables dialog) or by any ancestor (the properties panel scrolling all its sections at once), which is why the closest clipping ancestors are found in the DOM to know what is visible. The rows are also redesigned: a single line of a fixed height (instead of one or two lines depending on the available width), the type displayed with the compact select field used in the rest of the editor (or only its icon when the width is scarce), a smaller indentation and lighter text fields.
All the rows are now positioned by their index in the container holding all of them, instead of being laid out in a block positioned at the first visible row: this way a variable that is added at the end of a long list is not remounted - and does not lose the focus - when the list scrolls to it. The focus of a newly displayed variable is also given by the code scrolling to it, once its row exists, and the fields of unmounted rows are forgotten (the pointers of deleted variables get reused).
The properties panel can be resized, so whether the rows are laid out tightly (smaller indentation, type displayed as an icon only) now depends on the width the list is measured to have, instead of it being a compact list. Also add unit tests of the flattening of the variables into rows.
The box had the same color as the row, so it was invisible: it now only appears on hover, to show that clicking the icon of the type changes it.
|
Cursor Agent can help with this pull request. Just |
The labels of a row (the value of a boolean, the type of a variable that can't be edited) are plain elements, and the font is otherwise only set by the Text components: they were falling back to the font of the browser.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A scene with 500 variables made the properties panel (and the variables dialog) slow to display and slow to react to any change, because every row of the list was rendered. The list now renders only the rows that are on screen, and its rows were redesigned to be denser.
Rendering only the visible rows
The scroll of the variables list is not always handled by the list itself: in the properties panel, the panel scrolls all its sections at once. So instead of using a virtualized list component (which would own the scroll), the list keeps rendering into a container as tall as all its rows, and only fills it with the rows that are on screen:
FlattenVariables.jsflattens the tree of variables (both the inherited ones and the ones of the container) into the list of rows to display, applying the search filter and hiding the children of folded variables.useVisibleRowsRange.jscomputes which of these rows are on screen. It does not care about which ancestor scrolls: it walks up the DOM once to collect the ancestors that clip their content, and intersects their rectangles with the window to know the band of the container that can actually be seen. A capturingscrolllistener onwindowcatches the scroll of any of them, and aResizeObservercatches the panel being resized.index * VARIABLE_ROW_HEIGHTin that container, so a row keeps its DOM node when the visible range changes — which matters when a variable is added at the end of a long list: the list scrolls to it and its name field is focused, and it must not be remounted in between.Every row therefore has the exact same height, which is what makes a row's position computable without rendering the ones before it.
Denser and tidier rows
A row used to be laid out on one or two lines depending on the available width, with a text field showing a border at all times. It is now always a single line of 30px (instead of 36px in a dialog and 53px in the properties panel):
CompactSelectField, the select field used in the rest of the editor, and is reduced to its icon (with a tooltip, and a box appearing on hover) when the list is too narrow for the name of the type to fit;The variables of an instance in a dialog, before and after
The variables of a scene in the properties panel, before and after
Checks
500 variables (900 rows once the structures and arrays are expanded): 23 rows rendered instead of 900, and a change in the list (a keystroke, a type changed, a variable folded) takes ~20ms instead of ~2.1s.
variables_list_500_variables_scrolling_adding_and_searching.mp4
The Storybook stories got two additions used to check this:
WithManyVariables(a scene with 500 variables) andCompactWithManyVariablesInScrollingPanel(the same variables in a narrow panel which is the one handling the scroll). Both were driven in a real browser to check that scrolling anywhere in the list displays exactly the expected variables, that no more rows than needed are rendered, that all the rows have the same height, that the type and value columns stay aligned whatever the indentation, that adding a variable scrolls to it and focuses its name, that the search filters correctly, and that drag and drop still moves variables.FlattenVariables.spec.jscovers the flattening itself: the order of the rows, folded variables hiding their children, inherited variables being displayed first without the ones being overridden, and what a search displays.To show artifacts inline, enable in settings.