Skip to content

Dev.ej/better g2p toast#483

Closed
joanise wants to merge 4 commits intomainfrom
dev.ej/better-g2p-toast
Closed

Dev.ej/better g2p toast#483
joanise wants to merge 4 commits intomainfrom
dev.ej/better-g2p-toast

Conversation

@joanise
Copy link
Member

@joanise joanise commented Dec 10, 2025

PR Goal?

  • make the g2p error toast stay longer
  • enable copy-pasting from the g2p error toast by neither dismissing it on click nor on hover
  • clear all outstanding toasts on successful alignment, they're no longer relevant once we get to step 2
  • disable spell checking on the text area

Feedback sought?

Whether the updated app feels better

Priority?

medium

Tests added?

no

How to test?

enter asdf 123 asdf : 1234 in the text box, and see:

  • asdf is no longer underlined as a spelling error
  • when you click on go to next step, you're able to interact with the g2p error toast without making go away
  • when you fix your text and align again, all the no-longer relevant toasts are gone

Confidence?

high

Version change?

no

@semanticdiff-com
Copy link

semanticdiff-com bot commented Dec 10, 2025

Review changes with  SemanticDiff

Changed Files
File Status
  packages/studio-web/tests/test-commands.ts  54% smaller
  packages/studio-web/src/app/app.module.ts  47% smaller
  packages/studio-web/src/app/upload/upload.component.ts  32% smaller
  packages/studio-web/project.json  0% smaller
  packages/studio-web/src/app/upload/upload.component.html  0% smaller

@github-actions
Copy link
Contributor

github-actions bot commented Dec 10, 2025

PR Preview Action v1.6.3
Preview removed because the pull request was closed.
2025-12-12 20:04 UTC

Copy link
Collaborator

@marctessier marctessier left a comment

Choose a reason for hiding this comment

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

I got this toast at first below and I had the time to copy it into this message :-)

Text processing failed.
g2p could not be performed, please check your text or your language code. These words could not be converted from text to phonemes by g2p: ' 123 ', ' : ', ' 1234 '.

ASDF did not have a red squiggly red line under it in the text area :-)

After converting the numbers to words ex: 1 ==> one

It was still complaining about the ":"

Text processing failed.
g2p could not be performed, please check your text or your language code. These words could not be converted from text to phonemes by g2p: ' : '.

So I changed the language to "English" and it processed to the next step.

NOTE: => I did see a red and yellow/orange toast fly by the screen on the first try "very quickly" before going to step 2..

I clicked on step one to try that again... I re-click on next step and this time only the yellow/orange toast "with some content" flash on the screen... ( Unable to read what it's about / I tried a few times ;-) but it's a warning and we are probably better of going to the next step since it was a "Congratulations! Here's your ReadAlong!"

@joanise
Copy link
Member Author

joanise commented Dec 10, 2025

NOTE: => I did see a red and yellow/orange toast fly by the screen on the first try "very quickly" before going to step 2..

Oh, hum, that's a poor side effect of my change to dismiss toasts once you're on step 2. It's probably the alignment failure toasts, telling you it has to try again with relaxed parameters. I wasn't thinking about them flashing like that, but what you describe is not good. I'll have to think about a better solution than that... Thanks for flagging!

},
"update-browserslist": {
"command": "npx update-browserslist-db@latest"
"command": "npx -y update-browserslist-db@latest"
Copy link
Collaborator

@sergeleger sergeleger Dec 11, 2025

Choose a reason for hiding this comment

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

💯

I actually took note of this issue today! Glad it's fixed.

@@ -176,7 +176,10 @@ Please check it to make sure all words are spelled out completely, e.g. write "4
);
}
this.toastr.error(err.error.detail, $localize`Text processing failed.`, {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Something like this might help persist the alignment warnings:

this.textProcessingErrToast = this.toastr.error( ... ).toastId;

next: (progress) => {
if (progress.hypseg !== undefined) {
this.loading = false;
this.toastr.clear(); // clean all outstanding toasts on alignment success
Copy link
Collaborator

@sergeleger sergeleger Dec 11, 2025

Choose a reason for hiding this comment

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

and clear only the long-lived text processing error message.

            if (this.textProcessingErrToast) {
              this.toastr.clear(this.textProcessingErrToast); // clean all outstanding toasts on alignment success
              this.textProcessingErrToast = 0;
            }

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice, thanks for the suggestion on how to do this.

@sergeleger
Copy link
Collaborator

NOTE: => I did see a red and yellow/orange toast fly by the screen on the first try "very quickly" before going to step 2..

Oh, hum, that's a poor side effect of my change to dismiss toasts once you're on step 2. It's probably the alignment failure toasts, telling you it has to try again with relaxed parameters. I wasn't thinking about them flashing like that, but what you describe is not good. I'll have to think about a better solution than that... Thanks for flagging!

Here's the warning message I'm getting:
image

@sergeleger sergeleger self-requested a review December 11, 2025 17:25
Base automatically changed from dev.ej/ng20 to main December 12, 2025 17:34
@joanise
Copy link
Member Author

joanise commented Dec 12, 2025

NOTE: => I did see a red and yellow/orange toast fly by the screen on the first try "very quickly" before going to step 2..

Oh, hum, that's a poor side effect of my change to dismiss toasts once you're on step 2. It's probably the alignment failure toasts, telling you it has to try again with relaxed parameters. I wasn't thinking about them flashing like that, but what you describe is not good. I'll have to think about a better solution than that... Thanks for flagging!

Here's the warning message I'm getting: image

Oh, right, just processed this carefully: the problem is with English g2p: OOVs get mapped to nothing. So "This asdf test" gets g2p'd to the same thing as "This test", and we end up losing a word in the process. Instead of dropping the word, we map it using the "und" universal g2p mapping. This is expected given your input, and not a bug. And I guess it would be important not to hide it, the user might want to be able to read this.

@joanise
Copy link
Member Author

joanise commented Dec 12, 2025

Closing this without merging: I'm going to turn this into three separate PRs to merge independently, since one of the changes combined here has problems and isn't ready to go.

@joanise joanise closed this Dec 12, 2025
@joanise joanise deleted the dev.ej/better-g2p-toast branch December 19, 2025 20:35
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.

3 participants