SF-3825 Allow parallel editor view even when a source isn't set#3952
SF-3825 Allow parallel editor view even when a source isn't set#3952pmachapman wants to merge 1 commit into
Conversation
pmachapman
temporarily deployed
to
screenshot_diff
GitHub Actions
Inactive
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3952 +/- ##
==========================================
- Coverage 80.88% 80.87% -0.01%
==========================================
Files 634 634
Lines 41019 41033 +14
Branches 6674 6675 +1
==========================================
+ Hits 33179 33187 +8
- Misses 6789 6792 +3
- Partials 1051 1054 +3 ☔ View full report in Codecov by Harness. |
pmachapman
added
will require testing
There was a problem hiding this comment.
Pull request overview
Adds an “always on” left-hand tab pane in the translation editor so users can keep a parallel view even when no source project is configured, using a non-persisted “blank” tab as the empty-state UI.
Changes:
- Introduces a new
blank-tabeditor tab type (non-closeable / non-movable / non-persisted) with i18n header + notice text. - Updates editor tab-state behavior to ensure an empty tab group shows the blank tab, and adjusts small-screen consolidation/deconsolidation behavior accordingly.
- Simplifies editor layout logic by removing
showSource-gated UI behavior and updating affected unit tests.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json | Adds English strings for the blank tab header and empty-state notice. |
| src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/tabs/editor-tab-factory.service.ts | Adds factory support for creating the new blank-tab tab. |
| src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.ts | Implements blank-tab add/remove logic; updates consolidation logic and tab placement behavior. |
| src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.spec.ts | Updates/removed tests that depended on showSource behavior. |
| src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.scss | Adds styles for the blank tab notice container. |
| src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.html | Always renders the two-pane tab layout and adds template content for blank-tab. |
| src/SIL.XForge.Scripture/ClientApp/src/app/shared/sf-tab-group/tab-state/tab-state.service.ts | Makes deconsolidateTabGroups() return a boolean indicating whether deconsolidation occurred. |
| src/RealtimeServer/scriptureforge/models/editor-tab.ts | Adds blank-tab to the shared tab-type model and exports editorTabGroupTypes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .blank-tab-container { | ||
| position: absolute; | ||
| top: 50%; | ||
| left: 50%; | ||
| transform: translate(-50%, -50%); | ||
| width: 80%; | ||
| } |
| ? (this.tabState.getTabGroup(groupId)?.tabs ?? []).slice() | ||
| : tabs.filter(t => t.groupId === groupId); | ||
| if (groupTabs.length === 0 && blankTab == null) { | ||
| // Add the blank tab, if there are no tabs the tab group |
| /** | ||
| * Restores tabs moved from last consolidation into their original groups. | ||
| * | ||
| * @returns `true` if the tabs were deconsolidated, `false ` if there was no need to. |
| private addBlankTab(tabs: FlatTabInfo<EditorTabGroupType, EditorTabInfo>[] | undefined = undefined): void { | ||
| for (const groupId of this.visibleTabGroups) { | ||
| // For each tab group (i.e. source and target) | ||
| const blankTab = this.tabState.getFirstTabOfTypeIndex('blank-tab', groupId); | ||
| const groupTabs: EditorTabInfo[] = |
RaymondLuong3
left a comment
RaymondLuong3
left a comment
There was a problem hiding this comment.
I am getting an error at runtime. I am only able to test this behaviour if I hardcode values for editorTabGroupTypes.
It seems to me that is should be the responsibility of the tab group component to show the empty state instead of the host components to generate empty pages. Is this feasible?
@RaymondLuong3 reviewed 8 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on pmachapman).
src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.ts line 1454 at r1 (raw file):
: tabs.filter(t => t.groupId === groupId); if (groupTabs.length === 0 && blankTab == null) { // Add the blank tab, if there are no tabs the tab group
Nit: That should say 'if there are no tabs in the tab group'
Code quote:
// Add the blank tab, if there are no tabs the tab group
This PR adds an "always on" left hand tab pane, which when empty will show an "new tab" tab (I'm not sure on the wording/layout of this tab, so am really open to suggestions):
This change is