Repository navigation
feat(editor): Settings → General → Name the tab after the folder - #2536
Open
BenNewman100 wants to merge 2 commits into
Open
BenNewman100 wants to merge 2 commits into
BenNewman100 wants to merge 2 commits into
Conversation
An opt-in, cookie-only switch (folderTabNamesFolder, cookie plannotator-folder-tab-names-folder, default off). Off keeps the owner's ruling on backnotprop#2392: a folder session's tab names the open file, so several tabs of one folder stay apart (backnotprop#2061). On, the tab names the folder whichever file is open, `add-refund-flow/ · acme/payments`, as the header already does. - documentTabTitle: the folder rule, for folder sessions only; one file, several files and the last message are unchanged, and the repository label still follows "Hide the repository owner" - Settings: the General row, shown where a host passes showFolderTabSetting; Plannotator does in a folder annotate session - Docs: ui-settings.md, guides-and-shared-ui.md, ui HANDOFF - Tests: documentTitle with the setting on, the setting's default and cookie, a DOM test for the row (registered in tests/dom-test-list.txt), and the folder-session e2e spec turns it on through Settings and reloads
…ession
The row showed only in a folder session, so it could not be set from a
single file before a folder was opened. Every annotate session now offers
it, as "Hide the repository owner" shows everywhere; the row's text already
scopes it to folder reviews. Plan review still gets none.
- AppHeader passes showFolderTabSetting={annotateMode}, as it passes mode;
App's folderTabSettingAvailable prop is gone
- Help text: "the browser tab shows the folder name instead of the open file"
- Docs: ui-settings.md, guides-and-shared-ui.md, ui HANDOFF
- Tests: AppHeader DOM test, the row in a single-file annotate session and
none in plan review (registered in tests/dom-test-list.txt)
This branch has not been deployed
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.
This is the optional setting I offered in my comment on #2392.
Your ruling on #2392 stays the default: a folder session's tab names the open file, so several
tabs of one folder stay apart (#2061).
This adds a switch for people who review one folder per tab and want the tab to name the
subject of the review, which is usually the folder.
What changes
open a folder. Its text says it is for folder reviews. Plan review doesn't show it.
add-refund-flow/ · acme/payments-service.Plannotator, as every tab does.The row reads:
The same row in a single-file session. The tab strip is drawn, as explained below, and still names the file:
Before and after
A folder session with
proposal.mdopen. A page screenshot has no browser tab, so the stripalong the top is drawn: its text is the page's own
document.title, read at the moment of theshot.
Off (the default):
On:
After a reload with
design.mdopen, the tab still readsadd-refund-flow/ · acme/payments-service.Relation to #2537
A companion PR adds
--title <text>toannotate,annotate-lastandreview(#2289).It lets whoever starts the session name the tab.
That solves the same problem more generally, so it could land instead of this setting.
Both could land too:
If both land,
--titlewins over this setting. Both PRs editdocumentTabTitle, so whicheverlands second needs a small merge. I'll do it.
How
folderTabNamesFolderinpackages/ui/config/settings.ts: cookie-only(
plannotator-folder-tab-names-folder), default false, shaped likehideRepoOwner.documentTabTitle(packages/editor/documentTitle.ts) checks it first for a folder sessionthat has a folder path. Nothing else in the resolver changes.
SettingsgainsshowFolderTabSetting(default false): the row only stores the setting, so ahost opts in.
AppHeaderpassesannotateMode, so every annotate session shows it and planreview doesn't. Other hosts are unchanged.
ui-settings.md,guides-and-shared-ui.mdand the uiHANDOFF.md.Tests
Each one fails without the change it guards:
packages/editor/documentTitle.test.ts, with the setting on:all name the folder. With the rule removed, those four tests fail.
allowed past folder sessions, that test fails.
packages/ui/config/folderTabNamesFolderSetting.test.ts: off by default, and only an explicittrueorfalsecookie is read. With the default flipped on, it fails.packages/ui/components/Settings.folderTab.test.tsx(DOM, added totests/dom-test-list.txt):packages/editor/components/AppHeader.folderTabSetting.test.tsx(DOM, registered too):tests/e2e/folder-session-header.spec.tsnow also opens Settings, turns the switch on, checksthe tab names the folder, and reloads. Against a build where the switch doesn't reach the tab,
it fails with
Expected: "add-refund-flow/ · Plannotator",Received: "proposal.md · Plannotator".Verified on Windows 11
Settings.*.test.tsx,packages/ui/config,AppHeader*.test.tsxandDocBadges.test.tsx: 202 tests across 35 files, under--isolateas CI runs them.
folder-session-header.spec.tsanddoc-badges-fit.spec.tspass headless against thecompiled binary.
bun run build:review,bun run build:hookand the compiled binary build succeed.packages/ui/tsconfig.strict-consumer.json. The only errors are inpackages/inboxandapps/inbox, which this doesn't touch.Drafted by Ben's agent at his request.