search

diproche / webinterface

1 stars 0 forks source link

Add a variable that hinders the message from being displayed without … #121

Closed Korosensei42 closed 5 years ago

Korosensei42 commented 5 years ago

…userinput.

Currently the successmessage gets displayed when going on the webpage immediately, i.e. without the user doing anything. Assuming the user makes a correct proof, this could lead to confusion, since nothing changed at all.

This PR fixes this by introducing a variable hasStarted (boolean) to keep track if the "Prüfen"-Button was clicked already or not.

TimothyGillespie commented 5 years ago

It's true. It's not very nice that it will show "all is correct" as default. I didn't forget about it either. Just implementing this is not straight forward or easy, though I think I have two solutions which might be viable.

To show the issue: Observe that, if you go into the proofEditor anywhere and you press the check button it will display the success message. So far as intended. But if you switch to somewhere else it will display the success message now even if you didn't enter the button.

Example: Go to "Übungen" and select the dummy exercise. Just press the check proof Button and it will display the success message. Now go to "Freies Üben" and see that the success message is already displayed, because we don't load a new page and this variable is now true even if we switch the page via react router. Also we need to make this variable true every time the input is changed. So we need to put this too, when we have a function which checks for errors while typing if we want it to work this dynamically. It's possible but not as clean.

This variable would need to go into a state or a property. Since this is a controlled component the state would get overwritten all the time, so this isn't viable either. If we put it into a property we have to now change the logic and reenter some more lines whenever implementing the proofEditor.

So the variable idea would work like this, but it makes it a lot more complicated and bloated.

The other solutions that came to mind just now were:

  1. When opening the proofEditor have the issue list be initalized with an "empty issue" which will not get displayed, but since it contains an error it won't show the success message.
  2. Just check if the user input is empty. If yes don't display the success message (this one should have come to my mind way earlier)

The second version will also work with the embedded proofEditor since the userinput is still empty. There is just another function adding the before and after text to it before checking.

I'd personally prefer version two since it's easier and less confusing.

I know it's a lengthy answer considering how small it really is, but I just wanted to highlight the pitfall in react and the way it works.