zeeguu / web

Frontend for the zeeguu web application.
https://www.zeeguu.org
5 stars 9 forks source link

Refactoring NextNavigation #535

Open mircealungu opened 2 months ago

mircealungu commented 2 months ago

image

Ehi Tiago, could this be a sign that we should move the edit button away from the next navigation? Or rename NextNavigation to something else? Currently it feels wrong to pass the notifyWordChange to the NextNavigation which passes it on further to the edit box...

I think the right direction would be to move both the speech button and the edit button. We've already talked about the speech into the main exercise as underline. If we also move the edit box, we could have a design like this:

image

We can almost skip passing the exerciseBookmark down to the NextNavigation too... because as far as I can tell, it's mainly used for the Speech and for the EditBookmark dialog.

In any case, I would say, the first step could be moving the Edit bookmark button, so the button lives in the same component as the bookmarks that it edits.

Do you see what I mean?

P.S. Surely, we could also, just as well not do it now, but rather open an issue instead.

_Originally posted by @mircealungu in https://github.com/zeeguu/web/pull/519#discussion_r1752581129_

mircealungu commented 2 months ago

@tfnribeiro said:

So the NextNavigation originally was just to create a component to avoid repeating a lot of the same formatting for each of the exercises. I think the main reason was because some exercises had speech and other didn't have the speech bubble (Match vs Others). Eventually that was also where Merle has implemented the celebration modal logic/message of learning a word.

This means that the component which essentially was meant to just place these icons in a bottom row in a standard way has gotten much more complex than it should.

I agree with having the speech / edit in the text, and removing the pronounce buttons from the end, and that should make it easier to unpack the component.

Now, I think we should still keep this idea that the exercise renders what it has to render, but the Next / Congratulations / Auto-Pronounce these are all exercise independent and they just just be placed bellow the current exercise. This would mean that this logic should not be part of the Specific Exercise implementation, but rather controls to the ExerciseSession.

I think to do this well it will take a bit of time, there is a lot of things that would have to be moved, but it would be beneficial to make sense of the code.

Essentially, this:

image

Would be part of a component in the Exercise code, rather than in each exercise itself. We still have the issue of having to test if it's the Match, but maybe we can just have a generic "MultipleWordsProgressed" logic for the rendering of the congratulations box.

tfnribeiro commented 2 months ago

I have started looking into this, and I have a question - how do we ensure we don't merge words if a user for instance translates something close to the solution?

Would it maybe be easier that we show the word underneath the solution in the Foreign language where the users could click to hear it rather than in the context? It's just that the exercises usually disable pronounciation in the text - and then we have to create an exception for this one word.

In cases the word is shown in Danish it's easier, and they could just click the big word to hear it pronounced?

tfnribeiro commented 2 months ago

image

Like this maybe? If the prompt word was already in Danish, then the styling will turn into the clickable style and below the span is the translation.

tfnribeiro commented 2 months ago

In the case we would prompt with the Learning word it would look like this:

image

mircealungu commented 2 months ago

I have started looking into this, and I have a question - how do we ensure we don't merge words if a user for instance translates something close to the solution?

Would it maybe be easier that we show the word underneath the solution in the Foreign language where the users could click to hear it rather than in the context? It's just that the exercises usually disable pronounciation in the text - and then we have to create an exception for this one word.

In cases the word is shown in Danish it's easier, and they could just click the big word to hear it pronounced?

yes, this would be very fine I think. interactive text is already complicated and we should not complicate it more for this!

tfnribeiro commented 2 months ago

I had an idea while working on this and related to showing the progress in the bookmark - maybe it would be really nice if we had a "flashcard" design that is used throughout the exercises - which would contain the Original word and translation and the progress and a way to listen to the word again - maybe it takes a lot of real estate, but I think it could be a nice way to tie the Spaced Repetition together with the words :)

tfnribeiro commented 2 months ago

I will just tag the issue of the visual style (https://github.com/zeeguu/web/issues/504), as I think this change is going to overlap with what we discussed in that issue.