Skip to content

Ticket Table Split View PR Review

Ed Mozley edited this page Oct 29, 2026 · 2 revisions

Ticket table: full width, split view and pager - review of PR #223

βœ… Done - merged with fixes on 29 October 2026: @Dr4inder's commit, merged as it was, followed by one review commit of mine. It will be in 4.5.0. The design is sound and the code is careful; most of the fixes come from work that landed on main after the pull request was opened, not from the pull request itself. Two of my predictions turned out wrong once I checked, and are corrected below rather than quietly removed.

Written for @Dr4inder, who built it, and for anyone contributing to FreeITSM next. Same shape as the earlier reviews: your code does X, the problem is Y, the approach now is Z. Lessons that keep coming back are collected in Contributing.

PR What Verdict
#223 The ticket table uses the full width until a ticket is opened. The ticket then opens beside it with a resizable divider, or on its own with Back on a narrower desktop. The shared pager becomes a compact footer outside the table's scroller Merge with fixes

What it does

In Table mode the ticket list used to be squeezed into a 360px middle column, even with no ticket open. The reading pane next to it was empty and wasted the rest of the screen. With this pull request:

  • No ticket open: the table takes all the width the folder pane leaves.
  • A ticket open on a wide screen: the ticket opens beside the table. A divider between them can be dragged, or moved with the arrow keys, Home and End, and neither side can be squeezed below a usable width.
  • A ticket open on a narrower desktop (too narrow for both at a usable width): the ticket takes the whole width, and Back returns to the table.
  • Back keeps the table's page, filters and scroll position, and keeps the ticket as it was underneath - so a live task editor or a half-written reply is not rebuilt.
  • Phones are unchanged.
  • The pager, shared by every module's table, becomes a compact footer outside the table's horizontal scroller, with "1-50 of 101", "Page 1 of 3" and a labelled Rows per page.
  • The workspace's property fields now use the classic pane's typography and colours, so the confidential field's colours show there too.

The Messages list is untouched: everything is scoped to body.ticket-table-mode.


Keep - nearly all of it

  • It reuses the native renderer. Nothing is drawn twice. showDetail() is called from the places that already open a ticket, show the selection summary or clear the pane, and the comment says why: visibility must not depend on a successful API response. A failed load still shows the pane with its error.
  • Back asks before it leaves. backToList() goes through TicketWorkspace.canLeave(), so pending writes and unsaved drafts are respected. The tests prove both: a pending write holds Back, and cancelling the "discard draft?" question keeps the ticket open.
  • The divider is accessible. role="separator", aria-valuenow/min/max, keyboard steps, Home and End, and pointer capture so a fast drag does not lose the divider.
  • The minimums are real. 440px for the list and 600px for the ticket, with the narrow mode taking over when both cannot fit. That avoids a split nobody can use.
  • availableWidth() measures the computed width, not the hidden rectangle, with a comment saying the alternative oscillates between layouts. That is the kind of TRAP: worth writing down.
  • The tests have controls. The old layout fails the new width, pager and divider checks, and real pointer input is used because synthetic pointer events do not create pointer capture. That is the same lesson as the previous review.
  • Cache-busters were bumped in every caller of the shared files.

Change

# Finding Why Verdict
1 Two Back buttons in full screen, and Back leaves full screen behind the full-screen code's back main gained its own Back for full screen (#225) after this PR was opened Change
2 One merge conflict in tickets/index.php The same #225 work changed the action row and the cache-busters Change
3 The pager says "Page 1 of 3" in English in every language Its strings are in lang/en only, and the old pager was language-free ("1 / 3") Change
4 The divider forgets its position on every reload The split is kept for the page session only Change
5 Two visible changes outside Tickets The pager change reaches every module's table, and the workspace fields change their look Check, then keep
6 Small things Change

1. Two Back buttons. Your code adds #ticketTableBack to the action row, and backToList() removes body.ticket-popout itself. The problem: since this was opened, full screen has its own Back (#popoutBackBtn), the bar shows the ticket's reference, and Escape, Back and the shrink icon all leave through toggleTicketPopout(). That function also clears the saved full-screen preference and puts the keyboard back on the list. In Table mode in full screen both Backs would show side by side. Your Back would also take the class off without any of that, so the bar would keep the ticket's reference and the next ticket would open full screen again. The fix: your Back is hidden while full screen is on, because full screen's Back already does the job. backToList() leaves full screen through toggleTicketPopout() rather than removing the class. Escape in the split view does what your Back does, so the keyboard has the same way out at every level: full screen to split, then split to the full-width list.

2. The conflict. Only tickets/index.php conflicts: the action row line and the cache-buster versions. Both are resolved by keeping both sides and taking the newer version numbers.

3. Translations. Your code adds range and page to lang/en/common.php and three strings to lang/en/tickets.php. The problem: the old pager showed 1 / 3, which reads the same in every language. The new one shows English to a German or Dutch analyst until someone translates it, so the change makes 22 locales slightly worse. The fix: German, the one language that already had the pager's strings, now has all five, and stays at 100%. Correction: I said 22 locales would get slightly worse. They will not. The others have not reached the table's strings yet, so their pager was already English (Previous, Next, Rows per page) and only the "1 / 3" changes. They get the five strings with the rest of the table's when the translation pipeline reaches them.

4. The divider's position. Your code keeps splitRatio for the page session only, deliberately, with "no new saved preference". The problem: the Tickets page reloads often - after a setting changes, a company switch, F5 - and dragging the divider back every time gets old quickly. The fix: the ratio is remembered in the browser (localStorage), in a try/catch so a private window still works. A screen width is personal to a device, so the browser is the right place for it, not a server preference. Your clamping still applies when it is read back, so a ratio saved on a big monitor cannot squeeze a laptop.

5. Visible changes outside Tickets. Neither is wrong; both were looked at:

  • The pager. Correction: I expected every module's table to get the new footer. Only the Tickets table does, because it is the only one in server mode - the others load all their rows and draw no pager. Assets, Tasks, Changes, Calendar and Domains look exactly as before.
  • The workspace's property fields. Removing the workspace-only typography means its labels take the classic pane's style. The parity test proves they match, which is the point. The workspace itself opens beside the table as it should.

6. Small things.

  • A few lines in tickets-table.js hold two statements (event.preventDefault(); setWidth(...)). They get split, as the house style asks.
  • stopResize(), layout() and bindLayout() get one-line docblocks, like the functions around them.
  • The test file's dense layout matches the file it extends, so it stays as it is.

What it does not do

You may be thinking of this as Outlook's preview pane off. It isn't quite that:

  • In Table mode the reading pane only appears once a ticket is opened, which is close to Outlook with the reading pane off.
  • In Messages mode the reading pane is always there, as before.

A real Reading pane: Right / Off choice for both lists would be a separate, small change, and would reuse this pull request's Back and narrow mode. It is noted as a possible follow-up, not a reason to hold this one.


How it was checked

  • Your three suites, after my fixes: 91 checks, all passing - data-table-server.php 9, ticket-workspace.php 54, ticket-table-live.php 28 - on a throwaway install with its own database, schema brought up to date with scripts/db_verify_cli.php --apply. The development database here is real data, and your fixture's refusal to run without FREEITSM_TEST_DATABASE naming the database is exactly the right guard.
  • Full screen and the split together, in a real Chrome on two throwaway tickets: the table full width with nothing open; a ticket beside it; Enter to full screen with one Back; Escape back to the split with the keyboard on the row and the full-screen preference cleared; Escape again to the table alone; the table's Back called during full screen leaving it cleanly; the divider's position saved.
  • The other modules' tables, looked at: unchanged, as above.

See also: The ticket table Β· Ticket table - Developer Guide Β· Review of PRs #211-#213 Β· Contributing

FreeITSM

Getting Started

Modules

Multi-tenancy (planned)

Blue sky thinking

Bugs resolved

Links

Clone this wiki locally