fabd / kanji-koohii

A web application to help Japanese language learners remember the kanji.
https://kanji.koohii.com
GNU Affero General Public License v3.0
228 stars 21 forks source link

[mobile] Flashcard Review Cant close Story box until data returns. #106

Closed iveskins closed 7 years ago

iveskins commented 7 years ago

I noticed this because I have been using the site on the train recently, so I frequently loose internet connectivity in tunnels. In the Mobile view, in the Review mode I can continue to review cards even if my internet drops. If I tap on the view story button (top of page) to pop out the story box, but the call to get story data is never returned, the box just get stuck with the loading spinner animation. The close button on the story box does not show until the story data is returned. It would be nice if the close button was usable before the data is returned. In this way I could close the story box and continue reviewing cards.

iveskins commented 7 years ago

http://imgur.com/a/GF65g

maybe this response {"status":2,"props":{"dialogTitle":"Edit Story"},"html":"\n\n\n\n<form name=\"EditStory\" method=\"post\" action=\"\/study\/editstory\">\n <input type=\"hidden\" name=\"ucs_code\" value=\"30495\" \/> <input type=\"hidden\" name=\"reviewMode\" value=\"1\" \/>\n \n <div id=\"my-story\">\n <div class=\"padding rtkframe\">\n\n <div class=\"left\">\n <div class=\"framenum\" title=\"Frame number\">79<\/div>\n <div class=\"kanji onhover\"><span class=\"cj-k\" lang=\"ja\" xml:lang=\"ja\">\u771f<\/span><\/div>\n \n <div class=\"strokecount\" title=\"Stroke count\">[10]<br\/><span title=\"ON reading\" style=\"font-size:120%\"><span class=\"cj-k\" lang=\"ja\" xml:lang=\"ja\">\u30b7\u30f3<\/span><\/span><\/div>\n <\/div>\n\n <div class=\"right\">\n <div class=\"keyword\">\n <span class=\"JSEditKeyword\" title=\"Click to edit the keyword\" data-url=\"\/study\/editkeyword\/id\/30495\">true<\/span>\n <\/div>\n\n\n <div id=\"storybox\">\n\n <div id=\"storyedit\" style=\"display:none;\">\n\n \n <textarea name=\"txtStory\" id=\"frmStory\"><\/textarea>\n <div class=\"controls valign\">\n <div style=\"float:left;\">\n <input type=\"checkbox\" name=\"chkPublic\" value=\"1\" id=\"chkPublic\" \/> <label for=\"chkPublic\">Share this story<\/label> <\/div>\n <div style=\"float:right;\">\n <input type=\"submit\" name=\"doUpdate\" value=\"Save changes\" title=\"Save\/Update story\" \/> <input type=\"button\" id=\"storyedit_cancel\" value=\"Cancel\" name=\"cancel\" title=\"Cancel changes\" \/>\n <\/div>\n <div class=\"clear\"><\/div>\n <\/div>\n <\/div>\n \n <div id=\"storyview\" style=\"display:block;\">\n <div id=\"sv-textarea\" class=\"bookstyle\" title=\"Click to edit your story\" style=\"display:block;\">\n<div class=\"empty\">[ click here to enter your story ]<\/div> <\/div>\n\n\n <\/div>\n <\/div>\n\n <\/div>\n\n <\/div>\n <div class=\"bottom\"><\/div>\n <\/div>\n\n<\/form>\n\n<div class=\"uiBMenu\">\n <div class=\"uiBMenuItem\">\n <a class=\"uiFcBtnGreen JSDialogHide uiIBtn uiIBtnDefault\" href=\"#\"><span>Close<\/span><\/a> <\/div>\n<\/div>\n\n"}

The close button <div class=\"uiBMenu\">\n <div class=\"uiBMenuItem\">\n <a class=\"uiFcBtnGreen JSDialogHide uiIBtn uiIBtnDefault\" href=\"#\"><span>Close<\/span><\/a> <\/div>\n<\/div> could be included in the code from the start, not returned with the data?

fabd commented 7 years ago

Thanks for the bug report.

UX can be improved there. In the meantime can you confirm if you can close by tapping in the empty space below?

iveskins commented 7 years ago

Hi. Thanks for the reply. So it seems if I have no connection and the red (Oops! Error 0 "communication failure".) bar comes up, then I can tap in the space below to close the story box. selection_085 But in the case where my connection is just very slow but not really off line (like a phone that loses signal going into a tunnel that doesn't know it's offline yet) then the story box just sits in the waiting for data animation state, and even if I tap on the empty space it doesn't close. selection_087

I could reproduce this in chrome by setting the network speed in dev tools to 1kbs/down 20kbs/up. menu_086

fabd commented 7 years ago

I can't reproduce the background mask not closing the dialog.

So may as well look into adding an obvious way to close the dialog.

Putting in the large close button is not a straightfoward fix because that is part of the ajax response of the specific "Edit Story" dialog, wheres the dialog handling is a higher level "AjaxDialog". Although of course those buttons have a consistent look in other menus... but this UI was added at a later point and not handled at the lower level (AjaxDialog).

I could hardcode it so that when AjaxDialog "close" option is enabled, that it also adds a large mobile friendly close button.. if we assume the smaller edge-to-edge dialog variant can always have the space for a large "close" button.

A less hacky version would be to just enable the AjaxDialog's "close" option all the time. This displays the little red cross in the top right (as seen in the Dictionary dialog). Then I'll have to check also what happens if you close dialog and the response comes soon after, in case it creates an error.

fabd commented 7 years ago

Oh I see. If I set "offline" in chrome, then I can get this "communication failure". Indeed the shim/mask doesn't respond then. Probably because the setting up of the dialog doesn't work as intended. Although fixing that won't make the dialog closing obvious, so I will look into the solution above.

fabd commented 7 years ago

I haven't forgotten about this bug. I've been messing around with Docker for a week now and it's just not working for me. I can't even give a standard Docker install for contributors because OS X is slower and has numerous options.

So I just give up and will try again the simple MAMP package so I can use my host OS (os x) instead of working from within Ubuntu VM.

Then.. I will be looking at this again.

fabd commented 7 years ago

Hi @iveskins

Sorry it took so long to look at this bug.

I have uploaded a potential fix to https://**staging**(dot)koohii(dot)com please make sure you use this URL (just in case I broke something it is not pushed to the public site yet).

I could not reproduce the non-responsive mask. In Google Chrome emulator anyway a mouse click always closes the shim. I was able to reproduce this earlier. Go figure.

The potential fix on staging is that a Close button is displayed while the dialog is loading. Therefore you should always be able to close it. Hopefully, since I am not able to reproduce the error... I hope the code that adds the close button executes... otherwise I'll need to find a way to reproduce the bug.

If you are still available and have the spotty connection please update me if this is a working fix, thanks!

fabd commented 7 years ago

I'm updating LIVE with this change, à priori fixed.

iveskins commented 7 years ago

I cant really test it on the train, but I tried on the staging site and in chrome. When I load a flashcard, then throttle the connection, then click story, I could now close the story box with either the X or the empty space below. I don't see the green close button while loading, but I can use the red x. Seems to be fixed. Thank you.

fabd commented 7 years ago

Okay. If you can't reproduce the original bug then I'll assume it's fixed.

Make sure to go back to https://kanji.koohii.com as staging site is not always up to date.