Skip to content

Conversation

@joanise
Copy link
Member

@joanise joanise commented Sep 17, 2025

PR Goal?

Error messages due to g2p processing errors and not shown in a way that is realistic for users to handle. The goal of this PR is to propose a functional mock up of what such error highlighting could look like, to start a team discussion on what we want to implement as a solution.

Fixes?

First pass at fixing the fact that people need to contact us whenever the studio-web fails for them, and pretty much only a RAS team member is able to identify what typo caused the failure.

Related to ReadAlongs/Studio#27 which I was starting to solve by deleting words with empty g2p output, only to figure out in discussion with Del that that is actually a really bad idea. Instead, we came up with this plan to show the errors clearly.

Feedback sought?

  1. Do you like the general approach of showing the errors, highlighted in a bright box like this?

  2. If so, let's discuss in more detail how it should actually be rendered -- this PR is a first draft of the concept.

Priority?

Medium high because this is a recurring problem.

Tests added?

No. way too early for this! Let's talk about it first.

How to test?

Warning: The PR preview won't work!

Locally fetch and checkout:

  • Studio branch dev.ej/partial-xml-in-422-assemble
  • Studio-Web PR branch dev.del/feat-studio-g2p-error-feedback-component

Spin up the servers locally:

npx nx run-many --targets=serve,serve-web-api --projects=studio-web

And try this input text:

Colon : really should be ignored as punctuation, but "und" considers it a letter...
Numbers 234 should be spelled out!
Stray diacritics ̓  are a problem.

On clicking "Go to the next step", you will see:
image

Confidence?

Medium:

  • high that we need something like this
  • medium that this is how we want to render it

Version change?

This will require a version change of both Studio-Web and Studio, because rendering this text here required adding it to the 422 HTTPException generated by the /assemble endpoint. We can't bump Studio first, though, because the current S-Web with the updated Studio gives us even uglier toasts in the case of g2p errors.
So the order has to be:

  • merge this PR once we get consensus on the right rendering
  • release at least an S-Web patch, though probably more like a minor since it's a new feature, so 1.7.0
  • review, improve and merge dev.ej/partial-xml-in-422-assemble in Studio and create release 1.3.0 (current is 1.2.1)

@semanticdiff-com
Copy link

semanticdiff-com bot commented Sep 17, 2025

@joanise joanise requested review from deltork, dhdaines, marctessier, roedoejet and sergeleger and removed request for deltork and sergeleger September 17, 2025 20:04
@github-actions
Copy link
Contributor

github-actions bot commented Sep 17, 2025

PR Preview Action v1.6.2

🚀 View preview at
https://ReadAlongs.github.io/Studio-Web/pr-preview/pr-463/

Built to branch gh-pages at 2025-11-19 20:29 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@marctessier
Copy link
Collaborator

If we do not click on the "x" to close an / acknowledge error toast message.... If you correct the error and create a new one in the text and re-click to go to next step... The new Error will be display BUT the original error that was corrected will re-get displayed.

@marctessier
Copy link
Collaborator

Ok, I was speaking with Eric. After 30 second it does time out and will not re-display the same corrected message.

I like how we display the error page on the main page... Where we now have time to analyze the message. Also how the "error" is being highlighted make it much easier to my in the text.

@dhdaines
Copy link
Collaborator

Not having time to read and deal with the error message was one of the bigger problems with the old setup. I really like how this shows you where in the text the problems are, that's the key to a useful error message (even if the message itself doesn't necessarily make sense, at least you know where to look...)

Copy link
Collaborator

@roedoejet roedoejet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't reviewed the actual code, but as discussed in the meeting, I think this fix (which is a fantastic idea) really deserves to update our text area with something more editable (e.g. Quill or ProseMirror or something like that). Having an alternate text box with the errors introduces a bit of UI clutter and I think will result in a more confusing UX. I realize this is more work to introduce a different text area, but in this case, I think it's worth it. Great initiative on this work though, I expect there will be a lot of re-usability with this PR.

UPDATE, TO DECIDE:

Nov. 27 2025 we decided to use some sort of in-place error highlighting. We have three possible solutions in order of preference:

  1. Overlay a highlight layer behind the textarea. We like this option because it means we keep our current textarea and don't introduce dependencies.
<div class="wrapper">
  <pre id="highlightLayer"></pre>
  <textarea id="ta"></textarea>
</div>
  1. Use a contenteditable div. If the overlay doesn't work, we should do this because it doesn't introduce dependencies.
  2. Use a rich text editor like ngx-editor. This is a last resort because of the dependency bloat.

@sergeleger
Copy link
Collaborator

Here's the Gemini generated example of an overlay solution (<div/> on top of the <textarea />)

These are the words in the dictionary:

  • customword
  • anothercustom
  • gemini

overlay-example.html

@sergeleger
Copy link
Collaborator

To expand on my initial comment yesterday; quick fixes to improve the error message of the toast:

  • diacritics have defined Unicode ranges, so we could check if the error is due to a diacritic character and display the word "diacritic(s)" instead of the offending character.
  • diacritics look better when surrounded by spaces: "' " + diacritic + " '"

image compared to
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants