Skip to content

Fix result-variable column order and keep them on column reset - #182

Merged
ramonski merged 2 commits into
2.xfrom
result-variables-columns
Jul 17, 2026
Merged

ramonski merged 2 commits into
2.xfrom
result-variables-columns

Conversation

@xispa

@xispa xispa commented Jul 17, 2026 •

Copy link
Copy Markdown
Member

Description of the issue/feature this PR addresses

Analysis services that define result variables (interim fields) contribute extra columns to the analyses listing. These columns are dynamic: they don't exist in the static column set and are created server-side only while building the folderitems, then inserted at the Result/AdditionalValues position of each review_states[].columns.

The ReactJS listing, however, derives the visible column order from the localStorage-backed merged config (_merged_column_config → merge_column_config), which:

  1. tail-appends any server key not present in the stored config, in columns-dict key order — and dynamic columns sit last in that dict; and
  2. uses review_states[].columns (the correctly-ordered list) only as a membership set for the visibility filter, never for ordering.

As a result the columns are visible but always ordered last. A second, related defect: resetColumns restores @state.columns from @root_el.dataset.columns — the page-render DOM snapshot captured before the folderitems ran — which contains none of the dynamic columns.

Reproducible with any service that has result variables (independent of result type or add-on).

Current behavior before PR

  • Result-variable columns always render at the far right of the listing, off-screen until the user scrolls — regardless of the configured Sample view analysis columns order.
  • Clicking Reset columns makes the result-variable columns disappear entirely.

Desired behavior after PR is merged

  • Result-variable columns render in their server-defined slot (e.g. immediately before Result), honoring review_states[].columns.
  • Reset columns restores server defaults without dropping the dynamic columns.

Changes

  • merge_column_config inserts a server key absent from the stored config at its server-defined position — right after its nearest already-placed server neighbor (front if none) — instead of tail-appending. Genuinely-trailing add-on columns still land at the end; mid-list columns land in their slot. The caller must therefore pass server_columns in the intended display order.
  • _merged_column_config feeds the review-state column order into the merge (the columns dict alone can't express mid-list placement, since dynamic columns are keyed last).
  • resetColumns re-renders from the live @state.columns rather than the stale dataset.columns snapshot.
  • Added column_config unit tests for mid-list insertion, multiple new keys, leading key, and anchor-under-user-reorder. Rebuilt bundle.

--
I confirm I have tested the PR thoroughly and coded it according to PEP8
standards.

@xispa xispa changed the title Position dynamically-added result-variable columns at their server sl… Position dynamically-added result-variable columns at their server slot and keep them on column reset Jul 17, 2026
@xispa xispa changed the title Position dynamically-added result-variable columns at their server slot and keep them on column reset Order result-variable columns at their server slot; keep on reset Jul 17, 2026
@xispa xispa changed the title Order result-variable columns at their server slot; keep on reset Fix result-variable column order and keep them on column reset Jul 17, 2026
@xispa
xispa requested a review from ramonski July 17, 2026 12:38
@xispa xispa added the Bug 🐞 Something is broken label Jul 17, 2026

@ramonski ramonski left a comment

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.

Thanks! Encountered that but as well recently

@ramonski
ramonski merged commit d9c7439 into 2.x Jul 17, 2026
1 check passed
@ramonski
ramonski deleted the result-variables-columns branch July 17, 2026 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug 🐞 Something is broken

Development

Successfully merging this pull request may close these issues.

2 participants