ENG-2185 Move export settings out of Settings into the Export panel - #1374
Merged
trangdoan982 merged 8 commits intoSep 28, 2026
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
trangdoan982
commented
Aug 31, 2026
Contributor
There was a problem hiding this comment.
Devin Review found 1 potential issue.
2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
trangdoan982
force-pushed
the
eng-2185-move-export-settings-out-of-settings-into-the-export-panel
branch
from
August 31, 2026 17:19
28c63a1 to
a2f5284
Compare
trangdoan982
changed the base branch from
roam-setting-overhaul
to
eng-2186-grammar-nodes-drill-down-and-settings-navigation-primitive
August 31, 2026 17:19
Contributor
PR size/scope checkThis PR is over our review-size guideline.
Please split this into smaller PRs unless there is a clear reason the changes need to land together. If keeping it as one PR, please add a brief justification covering:
|
trangdoan982
force-pushed
the
eng-2185-move-export-settings-out-of-settings-into-the-export-panel
branch
from
September 3, 2026 20:30
d528b99 to
9d3d1ae
Compare
trangdoan982
force-pushed
the
eng-2185-move-export-settings-out-of-settings-into-the-export-panel
branch
from
September 4, 2026 21:57
9d3d1ae to
cfba308
Compare
trangdoan982
force-pushed
the
eng-2185-move-export-settings-out-of-settings-into-the-export-panel
branch
from
September 4, 2026 22:46
cfba308 to
478402c
Compare
trangdoan982
force-pushed
the
eng-2185-move-export-settings-out-of-settings-into-the-export-panel
branch
from
September 5, 2026 22:55
478402c to
7eaf604
Compare
trangdoan982
force-pushed
the
eng-2185-move-export-settings-out-of-settings-into-the-export-panel
branch
from
September 7, 2026 22:23
a0293d1 to
d29883b
Compare
trangdoan982
force-pushed
the
eng-2185-move-export-settings-out-of-settings-into-the-export-panel
branch
from
September 8, 2026 01:02
d29883b to
156347d
Compare
trangdoan982
force-pushed
the
eng-2185-move-export-settings-out-of-settings-into-the-export-panel
branch
from
September 17, 2026 19:59
f29d9c6 to
24c90a9
Compare
trangdoan982
force-pushed
the
eng-2185-move-export-settings-out-of-settings-into-the-export-panel
branch
from
September 17, 2026 20:18
a82e191 to
37df89d
Compare
trangdoan982
force-pushed
the
eng-2185-move-export-settings-out-of-settings-into-the-export-panel
branch
from
September 17, 2026 20:30
37df89d to
f7c8e77
Compare
trangdoan982
force-pushed
the
eng-2185-move-export-settings-out-of-settings-into-the-export-panel
branch
from
September 18, 2026 20:40
f7c8e77 to
037e56c
Compare
An error occurred while trying to automatically change base from
eng-2188-node-type-settings-are-spread-across-seven-inner-tabs-put
to
roam-setting-overhaul
September 28, 2026 14:42
The 7 export controls were used by exactly one surface but lived in Settings, so changing an option meant leaving the Export flow, opening Settings, and coming back. Move them into a collapsed "Export options" disclosure at the bottom of the Export dialog's Export tab, and remove the Advanced > Export tab that ENG-2186 parked in Settings for this change to take out. The controls, their storage keys, and the read path in getExportSettings are unchanged, so saved values persist and export behaviour is untouched. The advancedExport tab id goes with it, and the saved "discourse-graph-export" deep link now resolves to the default tab instead of a tab that no longer renders. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The image showed the export options inside the Settings dialog, with the vertical tab list and the pre-taxonomy tab names. Both the Export tab and that chrome are gone, so the screenshot contradicted the prose above it. Removed rather than replaced: the surrounding text already names where the options live, and no fresh capture of the Export dialog is available yet. The frontmatter close-up further down is a crop of the control itself, so it stays accurate and is kept. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…eads The number and select panels in Export options defer their block-prop write behind a short timer, so an option edited a moment before clicking Export was read at its previous value. Awaited rather than fired: the commit only starts the Roam block update, so a synchronous flush would not have been enough on its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review feedback on the Export dialog: the options toggle sits on the left of one row with "Exporting N results" and the discourse-context checkbox on the right, instead of taking a row of its own. The option panels are also seeded from a fresh settings read each time the section opens. Collapse unmounts them while closed, so reading once at dialog mount meant a long-lived dialog could show values that had since changed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Review feedback: the checkbox sits right-aligned on the line above; the line below is the Export options toggle on the left and the result count on the right. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ltip Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The flush and its registry move to ENG-2286, which blocks this ticket. Known gap until then: the export settings and the Export button share this dialog, so an option edited within ~350ms of pressing Export is read at its previous value. No unmount happens in that path, so the commit-on-unmount in ENG-2186 does not cover it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Base automatically changed from
eng-2188-node-type-settings-are-spread-across-seven-inner-tabs-put
to
roam-setting-overhaul
September 28, 2026 14:52
trangdoan982
force-pushed
the
eng-2185-move-export-settings-out-of-settings-into-the-export-panel
branch
from
September 28, 2026 15:06
037e56c to
b370327
Compare
trangdoan982
force-pushed
the
eng-2185-move-export-settings-out-of-settings-into-the-export-panel
branch
from
September 28, 2026 15:18
b370327 to
82e01e3
Compare
trangdoan982
deleted the
eng-2185-move-export-settings-out-of-settings-into-the-export-panel
branch
September 28, 2026 17:45
This branch was successfully 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.
https://entire.io/gh/DiscourseGraphs/discourse-graph/trails/39
https://www.loom.com/share/0c24f4b276f049d29f5c21bbeb67083a
https://www.loom.com/share/d794a7965df54225bf5b5bb78ac1dadb
Reviewer brief
Export optionsdisclosure at the bottom of the Export dialog'sExporttab. The Export tab is gone from Settings. Storage keys, thegetExportSettingsread path, and export behaviour are untouched, so saved values carry over.ExportOptionswas moved withgit mv, so it reads as a rename. The only content changes are the import paths (it is no longer insidesettings/) and the component name.Verification
pnpm install --frozen-lockfilethenpnpm ci:validatefrom the repo root: exit 0.check-typesacross 12 packages, 142 tests in 23 files passing.apps/roamfile: 0 errors.Export.tsx(40) andBlockPropSettingPanels.tsx(3) report the same warning counts before and after this change (stashed and re-ran to compare), so no new ones. Oneexhaustive-depswarning this change introduced inSettings.tsxis fixed rather than left.dist/extension.jscontains the new disclosure and posthog event, and no longer contains thediscourse-graph-exporttab id.Scope check
$scope-checkagainst the ENG ticket and final diff.Done When:apps/website/content/roam/guides/sharing-discourse-graph.mdsaid the options live on theExporttab of the configuration, which this change makes wrong.Local delegated full review
$dg-delegated-full-reviewwhen no other full-review workflow is available.Known gap: export can read a stale setting
The pending-write flush this PR used before (
await flushPendingSettingWrites()in the export path) has been removed. It and its registry are tracked in ENG-2286, which blocks this ticket.Until ENG-2286 lands, editing an export option and pressing Export within about 350 ms reads the option at its previous value. The export settings and the Export button share this dialog, so no unmount happens on that path and the commit-on-unmount landed in ENG-2186 (#1375) does not cover it.
The underlying debounce bug is ENG-2287.