Display morph type tokens in UI#2205
Conversation
github-actions
bot
added
the
💻 FW Lite
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Rebasing on top of #2192 again to pull in the renames that happened over there. |
rmunn
force-pushed
the
feat/morph-types-in-ui
branch
from
602774b to
decaf67
Compare
frontend/viewer/src/project/data/writing-system-service.svelte.ts
Outdated
Show resolved
Hide resolved
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
|
I've been looking into the failing Playwright tests. Here's what I've found so far:
Test is grabbing the first entry to use as a search filter. Now that first entry has become
Chooses the first entry to delete, then expects that entry to no longer be in the list. Thing is, it's choosing We might want a precondition that counts the number of entries that match the one chosen, and checks that the result after deletion is one fewer. ... Actually, this one is failing when I run it locally in the
Updates the first entry by prefixing it with At least, that's what I think is going on. It's hard to tell, because when I look at the Playwright trace, the visuals that it captured are from when the placeholders are still visible, rather than when the text has shown up. So I can't tell from the trace which entry it's actually capturing the text from. . Basically, I have made a little progress, but much of the Playwright tests' behavior remains a mystery for now. My next step is going to be to rebase onto the latest commit in |
rmunn
changed the title
|
It might be premature to work on the Playwright tests, actually. Maybe I should wait until we have the sorting and filtering story worked out, then see what happens with the tests. I do want to know what happened to those "8 removed" snapshots from Argos, though. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e7c3b1b7-c318-4fd8-93a6-77ba41b5def7
📒 Files selected for processing (7)
backend/FwLite/FwLiteShared/Services/MiniLcmJsInvokable.csfrontend/viewer/src/lib/dotnet-types/generated-types/FwLiteShared/Services/IMiniLcmJsInvokable.tsfrontend/viewer/src/lib/dotnet-types/index.tsfrontend/viewer/src/project/data/morph-types.svelte.tsfrontend/viewer/src/project/data/writing-system-service.svelte.tsfrontend/viewer/src/project/demo/demo-entry-data.tsfrontend/viewer/src/project/demo/in-memory-demo-api.ts
| headword(entry: ReadonlyDeep<IEntry>, ws?: string): string { | ||
| if (ws) { | ||
| return headword(entry, ws) || ''; | ||
| return this.#decorated(entry, ws) || ''; | ||
| } | ||
| return firstTruthy(this.vernacularNoAudio, ws => this.#decorated(entry, ws.wsId)) || ''; | ||
| } | ||
|
|
||
| return firstTruthy(this.vernacularNoAudio, ws => headword(entry, ws.wsId)) || ''; | ||
| #decorated(entry: ReadonlyDeep<IEntry>, ws: string): string | undefined { | ||
| // Citation forms should not be decorated with prefix/postfix tokens, only lexeme forms get decorated | ||
| return entry.citationForm[ws] || this.#morphTypesService.decorate(entry.lexemeForm[ws], entry.morphType); | ||
| } |
There was a problem hiding this comment.
Make the decorated headword searchable too.
After Line 137, the UI can show -a/a-, but the current demo search/filter path still matches only raw lexemeForm/citationForm values. That means users can now see headwords they cannot filter for, which is already the failing -a case called out in the PR notes. Please drive filtering from the same decorated headword representation.
Morph types now show leading/trailing tokens in headword, but do not yet have a dropdown for editing them in the entry UI.
Lexeme forms should be decorated with prefix/postfix tokens according to the morph type, but citation forms are meant as "overrides" and should be reproduced exactly as-is, without morph type tokens. This is the rule used by FLEx for how it displays words, so FW Lite should do the same. As a bonus, there is now only one `headword` function in the writing system service, instead of two functions with the same name that did two slightly different things.
We don't need all the morph types for testing, just a few of them.
Now that morph types show prefix and postfix tokens, "-a" was sorting before "-UPDATED-" so the updated entry was no longer the first one in the list.
myieye
force-pushed
the
feat/morph-types-in-ui
branch
from
8ff898e to
312b763
Compare
2 participants
Only implements part of #1848, but #1848 should remain open even when this is merged.
Currently merging into #2192, which has become our main branch for the Morph Types feature and will stay open until all parts of the feature are ready (thus allowing us to release other features while the Morph Types branch isn't ready yet).
Morph types now show in FW Lite UI, but do not yet have a dropdown to choose a different one. (Update: Decided in team meetings NOT to have a dropdown for changing morph types, at least not yet, because of all the complexity involved in switching from a MoAffixMsa to a MoStemMsa or vice versa in liblcm. There are corner cases that need tests and UI warnings like FieldWorks has, and the decision was made to postpone that feature. If someone needs to switch the morph type of an entry, they're probably a linguist who already has FW Classic, so they can do it in FW Classic and then sync).