Add PublishIn field to entry editor #2081
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis pull request introduces a new "publishIn" field to the entry editor system. It adds field configuration, a publications service layer, UI components for selecting publications via MultiSelect, mock API data, and translation entries across seven locales. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
Note Free review on us!CodeRabbit is offering free reviews until Fri Nov 07 2025 to showcase some of the refinements we've made. Comment |
github-actions
bot
added
the
💻 FW Lite
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
Fixed in commit 3524539. The publications service now follows the parts-of-speech pattern - using WritingSystemService to compute labels and sort by those labels (via Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
frontend/viewer/src/lib/in-memory-api-service.ts (1)
368-371: Stub looks good; drop redundant field and minor identity tweak optionalReturning concrete publications unblocks UI. You can remove
deletedAt: undefined(redundant if optional). Optionally hoist the list to a file/class constant to avoid a new array identity on each call.- return Promise.resolve([ - {id: '1', name: {en: 'Main Dictionary'}, deletedAt: undefined}, - {id: '2', name: {en: 'School Dictionary'}, deletedAt: undefined}, - ]); + return Promise.resolve([ + {id: '1', name: {en: 'Main Dictionary'}}, + {id: '2', name: {en: 'School Dictionary'}}, + ]);(Optional outside this hunk)
// At top-level in this file: const MOCK_PUBLICATIONS: IPublication[] = [ {id: '1', name: {en: 'Main Dictionary'}}, {id: '2', name: {en: 'School Dictionary'}}, ]; // And in getPublications(): return Promise.resolve(MOCK_PUBLICATIONS);frontend/viewer/src/locales/es.po (1)
937-940: Provide an initial Spanish translation for this new keySuggest “Publicar en” to match UI context.
msgid "Publish in" -msgstr "" +msgstr "Publicar en"frontend/viewer/src/locales/id.po (1)
937-940: Add Indonesian translationSuggest “Terbitkan di”.
msgid "Publish in" -msgstr "" +msgstr "Terbitkan di"frontend/viewer/src/locales/ko.po (1)
937-940: Add Korean translationSuggest “게시 대상”.
msgid "Publish in" -msgstr "" +msgstr "게시 대상"frontend/viewer/src/locales/fr.po (1)
937-940: Add French translationSuggest “Publier dans”.
msgid "Publish in" -msgstr "" +msgstr "Publier dans"frontend/viewer/src/locales/sw.po (1)
937-940: Add Swahili translationSuggest “Chapisha katika”.
msgid "Publish in" -msgstr "" +msgstr "Chapisha katika"frontend/viewer/src/lib/entry-editor/object-editors/EntryEditorPrimitive.svelte (1)
128-140: Consider using pre-computed labels for efficiency.The implementation is functionally correct and follows the established MultiSelect pattern. However,
publications.currentalready includes computed labels (seeLabeledPublicationtype in publications.ts, line 6), so thelabelSelectoris recomputing labels unnecessarily.Apply this diff to use the pre-computed labels:
<MultiSelect onchange={() => onFieldChanged('publishIn')} bind:values={entry.publishIn} options={publications.current} - labelSelector={(pub) => publications.getLabel(pub)} + labelSelector={(pub) => pub.label} idSelector="id" sortValuesBy="selectionOrder" {readonly} />This eliminates redundant label computation and ensures consistent labels by using the values computed during sorting in
publications.current.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
frontend/viewer/src/lib/entry-editor/field-data.ts(1 hunks)frontend/viewer/src/lib/entry-editor/object-editors/EntryEditorPrimitive.svelte(3 hunks)frontend/viewer/src/lib/in-memory-api-service.ts(1 hunks)frontend/viewer/src/lib/publications.ts(1 hunks)frontend/viewer/src/lib/views/view-data.ts(1 hunks)frontend/viewer/src/locales/en.po(1 hunks)frontend/viewer/src/locales/es.po(1 hunks)frontend/viewer/src/locales/fr.po(1 hunks)frontend/viewer/src/locales/id.po(1 hunks)frontend/viewer/src/locales/ko.po(1 hunks)frontend/viewer/src/locales/ms.po(1 hunks)frontend/viewer/src/locales/sw.po(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-02T14:27:02.745Z
Learnt from: myieye
Repo: sillsdev/languageforge-lexbox PR: 1720
File: frontend/viewer/src/locales/es.json:1786-1790
Timestamp: 2025-06-02T14:27:02.745Z
Learning: Spanish locale file (frontend/viewer/src/locales/es.json) contains generated code that may have null line numbers in origin entries due to limitations in the code generation process.
Applied to files:
frontend/viewer/src/locales/es.po
📚 Learning: 2025-07-30T04:53:41.702Z
Learnt from: rmunn
Repo: sillsdev/languageforge-lexbox PR: 1844
File: frontend/viewer/src/lib/entry-editor/ItemListItem.svelte:26-37
Timestamp: 2025-07-30T04:53:41.702Z
Learning: In frontend/viewer/src/lib/entry-editor/ItemListItem.svelte, the TODO comments for unused props `index` and `actions` are intentional reminders for future work to be completed in a separate PR, not issues to be resolved immediately. These represent planned functionality that will be implemented later.
Applied to files:
frontend/viewer/src/lib/entry-editor/object-editors/EntryEditorPrimitive.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Build UI / publish-ui
- GitHub Check: Build API / publish-api
- GitHub Check: Build FW Lite and run tests
- GitHub Check: frontend
- GitHub Check: check-and-lint
- GitHub Check: frontend-component-unit-tests
- GitHub Check: Analyze (csharp)
🔇 Additional comments (7)
frontend/viewer/src/lib/entry-editor/field-data.ts (1)
18-18: Add looks correct; please verify help link pathHelp ID path format matches neighbors, but the segment
Publication_Settings_fldsdiffers from typicalEntry_level_fields/Sense_level_fields. Confirm it resolves to the intended doc.frontend/viewer/src/locales/en.po (1)
932-935: LGTMEnglish translation present and consistent.
frontend/viewer/src/lib/views/view-data.ts (1)
19-19: LGTM! Field configuration aligns with requirements.The
publishInfield is correctly configured withshow: falseandorder: 8, ensuring it's hidden in FieldWorks Lite but visible in FieldWorks Classic (via the[defaultDef]: {show: true}spread on line 47). The order places it as the last entry field before senses, as specified in the PR objectives.frontend/viewer/src/locales/ms.po (1)
937-939: LGTM! Translation key properly added.The new "Publish in" translation entry follows the standard localization pattern. The empty
msgstris expected and will be populated by translators in a future update.frontend/viewer/src/lib/entry-editor/object-editors/EntryEditorPrimitive.svelte (1)
12-12: LGTM! Service initialization follows established patterns.The import and initialization of the publications service is consistent with the complexFormTypes service pattern used elsewhere in this component.
Also applies to: 36-36
frontend/viewer/src/lib/publications.ts (2)
1-15: LGTM! Hook implementation follows established service patterns.The
usePublicationshook correctly implements the singleton pattern usingSymbol.forandgetOrAdd, consistent with other service initializations in the codebase. The type definition forLabeledPublicationappropriately extendsIPublicationwith a computed label.
17-39: LGTM! Service structure is clean and efficient.The
PublicationServicecorrectly:
- Fetches publications using the project context's API resource
- Computes labels once within the reactive
$derived.byblock and reuses them for sorting- Uses
localeComparefor internationalization-friendly sorting- Provides a
refetchmethod for data refreshThe implementation aligns with the PR description's goal of computing labels once and reusing them for consistency.
Co-authored-by: myieye <12587509+myieye@users.noreply.github.com>
Co-authored-by: myieye <12587509+myieye@users.noreply.github.com>
Co-authored-by: myieye <12587509+myieye@users.noreply.github.com>
myieye
force-pushed
the
copilot/add-publishin-to-entry-editor
branch
from
3524539 to
2320e2e
Compare
frontend/viewer/src/lib/entry-editor/object-editors/EntryEditorPrimitive.svelte
Outdated
Show resolved
Hide resolved
Resolves #2080
Plan: Add PublishIn to Entry Editor ✅ COMPLETE
Understanding from exploration:
publishInfield already exists in IEntry interface asIPublication[]getPublications()API method already exists in IMiniLcmJsInvokableTasks:
parts-of-speech.svelte.tspublishInfield to field-data.ts with help IDpublishInfield to view-data.ts (show only in FW_CLASSIC_VIEW)Recent Update (commit: Use WritingSystemService for publication sorting and labels)
Updated publications service to follow the parts-of-speech pattern:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.