Ticket Back Full Screen PR Review

Back from a full-screen ticket - PR review

PR #289 by Dr4inder · Follows #225 · Verdict: merge + fix

Back and Escape on a full-screen ticket now go straight back to the ticket list and leave full screen switched on, so the next ticket opens full screen again. User pages: Tickets § Pop-Out (Full-Screen) View and The ticket table. Internals: Ticket table - Developer Guide.

The problem it fixed

Back and Escape called toggleTicketPopout(), which is the layout switch: it saved tickets_popout = 0. So an analyst who worked full screen pressed Back once and the next ticket opened in the three-pane view. In Table mode it was worse: Back stopped at the ticket beside the table, and it took a second press to reach the list.

What the PR does

Piece (assets/js/inbox.js unless named)	What it does
backToTicketList()	What Back and Escape now call. In Table mode it hands over to TicketTable.backToList(), otherwise leaveTicketPopout()
leaveTicketPopout()	Sets ticketPopoutPaused = true and takes the class off through syncPopoutToTicketState(false). It never writes the saved preference
ticketPopoutPaused	"Back was pressed - show the list until somebody opens a ticket on purpose". syncPopoutToTicketState() returns early (popout off) while it is set; toggleTicketPopout() and any deliberate open clear it
The shrink icon	Still toggleTicketPopout() - the one explicit way to switch full screen off
selectEmail() "retained" path	Reopening the ticket you just left, while paused or while the table is shown alone, reselects it and re-enters full screen without fetching or redrawing, so its DOM and any half-written reply survive
selectEmail(id, {refresh: true}), loadTicketById(id, {refresh: true})	Every redraw that follows an action (move company, link / unlink problem, change or ticket, subject, send, wake, drop-assign, workspace switch, escalate) passes refresh. A refresh does not clear the pause or reopen the detail, so a reply arriving after Back cannot pop the ticket back over the list
TicketTable.backToList() (assets/js/tickets-table.js)	No longer awaits TicketWorkspace.canLeave() - it only hides the retained editor, so there is nothing to discard. It still waits while ticketPendingWrites > 0. canLeave() is asked when a different ticket is opened
enterRow(), openRowFullScreen()	Enter opens the ticket in the saved layout; Enter or double-click on the ticket already showing makes it full screen
tests/ticket-back-navigation.php	A browser test of the above

The risk: live updates

The PR was branched before live updates (#2795, #267; LivePulse in inbox.js) went in, so both changed the same file in ways neither had seen.

• The text conflict was only the ?v= numbers in tickets/index.php. Resolved as inbox.js?v=195, tickets-table.js?v=6.
• The real question was semantic. Live updates redraw the open ticket when a colleague changes it: liveReloadTicket() → displayEmail() → syncPopoutToTicketState(true). Without the pause that would put an analyst who had pressed Back straight back into full screen whenever somebody else touched the ticket. The PR's early return in syncPopoutToTicketState() while ticketPopoutPaused is set covers it, so a colleague's change never pops the ticket back over the list.
• Every other open. I went through every remaining selectEmail / loadTicketById call without {refresh: true}. All are deliberate opens - a click, Enter, the arrows, a URL, right-click Open - which should clear the pause.

What was checked

Question	How	Result
Does the PR's own test pass on the merged code (with live updates)?	tests/ticket-back-navigation.php on the test install	110/110
The ticket table's live test, which the PR had not run	tests/ticket-table-live.php	28/28 (its draft check now expects Back to keep the draft with no discard prompt)
Live updates	tests/live-pulse.php	22/22
Wider ticket screen	tests/ticket-workspace-live.php	One failure, native internal note saves in the same ticket. It fails the same way on v4.12.0, so it was not caused by this PR - probably the test typing into a note box that is now a rich-text editor

What was changed

• #2812 - the in-app help tickets.help ... fullscreen_dbl and fullscreen_back said Enter and Escape step in and out "one level at a time". Rewritten for the new behaviour in all six languages: Enter opens in the layout you last chose; Back and Escape return straight to the list; neither switches full screen off (the shrink icon does); going back to the ticket finds a started reply still there. Spanish, Indonesian and Polish had an older, shorter version of these two lines that the coverage audit could not see, because the keys existed - they now match English.

Merged with git merge --no-ff under the contributor's name (97efa5418), then the follow-up commit (713210a15).

Worth knowing

• 🔴 TRAP: any new code that redraws the open ticket after an action must pass {refresh: true} to selectEmail() / loadTicketById(). Without it the call counts as a deliberate open: it clears ticketPopoutPaused and brings a ticket the analyst has just left back over the list.
• 🔴 TRAP: anything that re-enters full screen must respect ticketPopoutPaused. Go through syncPopoutToTicketState(), never add the ticket-popout class directly.
• Back is navigation, not a setting. Do not route Back or Escape through toggleTicketPopout() again - that is what saved the preference off.
• Back keeps the ticket's DOM. Reopening the same ticket takes the "retained" path and must not fetch and redraw, or a half-written reply is lost. canLeave() belongs to opening a different ticket, not to Back.