Skip to content

Sheets: add "Data has header row" toggle to preserve headers during sort#495

Open
the-narwhal wants to merge 4638 commits into
ProtonMail:mainfrom
the-narwhal:sort-respecting-header-rows
Open

Sheets: add "Data has header row" toggle to preserve headers during sort#495
the-narwhal wants to merge 4638 commits into
ProtonMail:mainfrom
the-narwhal:sort-respecting-header-rows

Conversation

@the-narwhal

Copy link
Copy Markdown
Contributor

⚠️ Important note

Due to the constraints of working outside the full Proton development environment, I was not able to build or run the application locally to verify this end-to-end. The implementation follows all established codebase patterns and passes TypeScript diagnostics, but it has not gone through a full build pipeline or staging environment test. This is intended as a reference implementation Proton is welcome to merge, incorporate, or iterate on it as needed.


Summary

Adds a "Data has header row" toggle to the Sort submenu (Data → Sort) in Proton Sheets. When enabled, row 1 is excluded from sort operations so column headers stay in place. When disabled (the default), sort behavior is identical to before, no regression for existing users.

This addresses a community feature request where using "Sort A to Z" or "Sort Z to A" on a column would sort the entire sheet including row 1, destroying header labels.


What changed

ui-state.ts core logic

  • Adds hasHeaderRow: boolean state (default false, so existing sort behaviour is completely unchanged unless the user explicitly enables the toggle)
  • Adds toggleHeaderRow a stable event ref via useEvent() that flips the flag
  • Branches sortAscending and sortDescending: when hasHeaderRow is true, calls onSortRange with a range starting at row 2 instead of onSortColumn on the full column; when false, falls through to the original call
  • Exposes both hasHeaderRow and toggleHeaderRow on the data slice of the UI state so every sort entry point (menu bar, context menu, legacy context menu) shares a single source of truth without any duplicated logic

sort-utils.ts (new file)

Extracts makeSortRangeExcludingHeader(rowCount, columnCount) as a pure function with no @rowsncolumns/* dependencies. This keeps the range-building logic independently testable and documents the intentional edge case (a sheet with only one row produces an empty range callers treat it as a no-op).

sort-utils.test.ts (new file)

8 unit tests covering:

  • startRowIndex is always 2 regardless of sheet size
  • startColumnIndex is always 1
  • endRowIndex / endColumnIndex reflect the provided dimensions
  • Full-range snapshot for a typical sheet
  • Edge: single-row sheet (header-only, empty sort range)
  • Edge: single-column sheet
  • Edge: very large sheet (1 000 000 rows, 1 000 columns)

These follow the existing test pattern in the codebase (pure utility functions only, no component harness needed).

DataMenu.tsx new UI toggle

  • Adds a separator and a "Data has header row" menu item at the bottom of the Sort submenu
  • Uses selectedIndicator={hasHeaderRow} for the checkmark, matching the pattern used in the View menu
  • leadingIndent keeps it visually nested under the sort actions
  • Disabled in read-only mode
  • i18n string registered with the correct sheets_2025:Spreadsheet editor menubar data menu (sort submenu) context

ContextMenu.tsx and LegacyContextMenu.tsx routing + hooks fix

  • Extracts stable sortAscending and sortDescending refs at component top level (same pattern already used for insertLink in both files)
  • This also fixes a latent Rules-of-Hooks violation previously the sort onClick handlers were inline arrow functions calling useUI.$ inside JSX, which is not valid under React's hooks rules
  • Both "Sort A to Z" and "Sort Z to A" buttons now go through useUI.$.data.sortAscending / sortDescending, so the hasHeaderRow check in ui-state.ts is respected from every sort trigger in both the new and legacy UI

Behaviour

State "Sort A to Z" / "Sort Z to A"
Toggle off (default) Sorts entire column including row 1 — identical to current behaviour
Toggle on Calls onSortRange with startRowIndex: 2, keeping row 1 fixed

The toggle is session-local (resets on page reload). Persisting it to user preferences would be a natural follow-up.


What has and hasn't been tested

Status
TypeScript diagnostics (pre-existing errors only, no new ones)
makeSortRangeExcludingHeader unit tests (8 cases) ✅ Written, structured to pass
Full Jest test run ❌ No local node_modules available
Build pipeline ❌ Not run
Browser / staging smoke test ❌ Not possible in this environment

Suggested manual verification steps

  1. Open any sheet with labelled column headers in row 1
  2. Click a cell in a data column
  3. Data → Sort → Sort A to Z confirm row 1 sorts with the data (default behaviour, no regression)
  4. Data → Sort → Data has header row confirm a checkmark appears
  5. Data → Sort → Sort A to Z confirm row 1 stays in place; rows 2 onward sort
  6. Data → Sort → Sort Z to A same check, descending
  7. Right-click a column → Sort A to Z (context menu) confirm toggle is respected
  8. Click "Data has header row" again confirm checkmark disappears and full-column sort resumes
  9. Open a read-only sheet confirm all sort items including the toggle are disabled

Raphael Rychetsky and others added 30 commits April 21, 2026 09:15
Prevent concurrent referral spotlight feature PUTs

See merge request web/clients!24282
VPNPLG-58: Add payload and telemetry when forking the session

See merge request web/clients!24135
fix: fix icon alignment in cancellation flow

See merge request web/clients!24315
Add offer card UI components

See merge request web/clients!24302
added temporary need content and config

See merge request web/clients!24307
feat: workspace premium cancelation flow

See merge request web/clients!24319
Add news subscription for Meet

See merge request web/clients!24209
use the same setting menu for extension as for web

See merge request web/clients!24241
INWEB-867: Add common setup for Slack API in the CI

See merge request web/clients!24271
Add new permissions modals and stop requesting permissions automatically

See merge request web/clients!24248
DRVWEB: Search blobs encryption/decryption

See merge request web/clients!23888
[IDTEAM-5613] Add telemetry for sharing

See merge request web/clients!24221
Limit sharing settings visibility only to admins on parent share

See merge request web/clients!24326
[DRVWEB-4974] Album listing sdk

See merge request web/clients!23946
Enable telemetry for Meet

See merge request web/clients!24331
MargeBot and others added 20 commits April 24, 2026 09:29
Prevent missing navigator.mediaDevices to throw

See merge request web/clients!24429
Update category counts when labelling a message from a conversation

See merge request web/clients!24325
Update openSubscriptionModal usage

See merge request web/clients!24107
Fix a bug storing feature flags in cookie

See merge request web/clients!24454
[DRVWEB-4967] delete legacy drive view

See merge request web/clients!24263
NOISSUE: Adapt telemetry for tv flow

See merge request web/clients!24405
Defer update prompt if user is in meeting

See merge request web/clients!24450
Mail BYOE: hide lock icons in sent messages when `X-Pm-Byoe` header is present

See merge request web/clients!24000
PostQuantumOptInModal: prompt for auth before committing the opt-in

See merge request web/clients!24457
Add LoginLink member creation mode

See merge request web/clients!24448
Sorting a column via "Sort A to Z" / "Sort Z to A" was sorting the
entire sheet including row 1, destroying column header labels. This
adds an opt-in toggle under Data → Sort that, when enabled, excludes
row 1 from all sort operations.

Changes
-------
ui-state.ts
- Add `hasHeaderRow` boolean state (default false — existing behaviour
  unchanged unless the user explicitly enables the toggle)
- Add `toggleHeaderRow` event setter (stable ref via `useEvent`)
- Branch `sortAscending` / `sortDescending`: when `hasHeaderRow` is
  true, call `onSortRange` with a range starting at row 2 instead of
  calling `onSortColumn` for the full column
- Expose `hasHeaderRow` and `toggleHeaderRow` on the `data` slice of
  the UI state so all sort entry points share a single source of truth

sort-utils.ts (new)
- Extract `makeSortRangeExcludingHeader(rowCount, columnCount)` as a
  pure function so the range-building logic can be unit-tested without
  importing @rowsncolumns/* packages

sort-utils.test.ts (new)
- 8 unit tests covering typical sheets, single-column sheets,
  header-only sheets (edge: empty range), and large sheets

DataMenu.tsx
- Add "Data has header row" menu item inside the Sort submenu with a
  checkmark indicator and proper i18n context string

ContextMenu.tsx / LegacyContextMenu.tsx
- Extract stable `sortAscending` / `sortDescending` refs at component
  top level (fixes implicit Rules-of-Hooks violation from inline hooks
  inside JSX event props)
- Route "Sort A to Z" / "Sort Z to A" buttons through
  `useUI.$.data.sortAscending` / `sortDescending` so the header-row
  toggle is respected in both new and legacy UI modes
@the-narwhal the-narwhal marked this pull request as ready for review April 26, 2026 17:29
@mmso mmso force-pushed the main branch 9 times, most recently from 3a2ffcb to 8b49cee Compare June 9, 2026 09:23
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.