search

TIM-JYU / TIM

TIM (The Interactive Material) is an open-source cloud-based platform for creating interactive learning documents.
https://tim.education/view/about/en-US
MIT License
14 stars 4 forks source link

Jotkin painikkeet vaativat tuplanaputuksen Safari-selaimessa iOS-laitteilla - [closed] #2697

Closed dezhidki closed 2 years ago

dezhidki commented 4 years ago

Merges fix-tablet-click -> master

Liittyy seuraavaan ongelmaan

iPadin kahden painamisen vaativa ongelma = iPadissä pitää painikkeita painaa 2x

Tämä korjaus on toistaiseksi ainoastaan AngularJS-komponenteille, koska Angular 9 -komponenttien pitäisi toimia iOS-laitteillakin hyvin. Tämän pitäisi siis mahdollisesti riittää lähiviikkojen tarpeisiin.

Testiversio on viety testipalvelimelle: https://timdevs01-2.it.jyu.fi/view/users/test-user-1/cstest (käyttäjälle testuser1).

Testattu usealla iOS-laitteella (iPad 7, iPad Pro 2017 & 2018), ja näyttäisi toimivan hyvin (tosin en testannut vielä kaikkia mahdollisia plugineja). Chromella ja Firefoxilla ei vastaavia ongelmia havaittu.

dezhidki commented 4 years ago

In GitLab by @vesal on May 6, 2020, 20:49

Tämä korjaus on toistaiseksi ainoastaan AngularJS-komponenteille, koska Angular 9 -komponenttien pitäisi toimia iOS-laitteillakin hyvin.

Mikä vika siellä oli?

Vesa

dezhidki commented 4 years ago

Lyhyesti selitettynä se, että näköjään AngularJS:n ngClick-direktiivi on laitettu kuuntelemaan vain click-tapahtumia. Chromella ja Firefoxilla se on OK, sillä tableteilla ne tuottavat click-tapahtumia, mutta Safari ensisijaisesti tuottaa touch* (touchstart, touchend, yms) tapahtumia ennen clickia. Safarilla click tulee vasta, kun elementti on fokuksessa.

Avasin hieman tarkemmin tuon koodissa, mutta tässä vielä linkit, jonka perusteella kirjoitin korjauksen:

Näyttäisi siis olevan AngularJS:n juttu, sillä en löytänyt vastaavia "ongelmaraportteja" Angular 9:lle.

dezhidki commented 4 years ago

In GitLab by @vesal on May 6, 2020, 21:00

Lyhyesti selitettynä se, että näköjään AngularJS:n ngClick-direktiivi on laitettu kuuntelemaan vain click-tapahtumia. Chromella ja Firefoxilla se on OK, sillä tableteilla ne tuottavat click-tapahtumia, mutta Safari ensisijaisesti tuottaa touch* (touchstart, touchend, yms) tapahtumia ennen clickia.

Hyvä kun selvisi :-)

Vesa

dezhidki commented 4 years ago

In GitLab by @vesal on May 6, 2020, 21:03

Lyhyesti selitettynä se, että näköjään AngularJS:n ngClick-direktiivi on laitettu kuuntelemaan vain click-tapahtumia. Chromella ja Firefoxilla se on OK, sillä tableteilla ne tuottavat click-tapahtumia, mutta Safari ensisijaisesti tuottaa touch* (touchstart, touchend, yms) tapahtumia ennen clickia.

Periaatteessahan tuo ei nähtäväsi moneessa kohti kokeessa olisi aiheuttanut ongelmai, koska esim qst klikataan ensin jotakin valintakohtaa ja se näköjään tulee valituksi yhdellä klikkauksella. Sitten focus taitaakin jo olla divissä jossa on Tallenna ja silloin se toimii ekalla.

Mutta jos rupee m iettimään ja ruksii ensin useita eri kysymyksiä ja vasta sitten menee tallentamaan, niin tuo voi yllättää, siksi hyvä jos korjaantui :-)

Myös csPlugin "normaalikäytössä", eli en focus edit arean, kirjoittaminen ja Tallenna taitaa toimia ilmankin muutoksia. Mutta taas jos käy muualla välillä, voi tulla yllätys kun ei tallennukkaan.

dezhidki commented 4 years ago

In GitLab by @Smibu on May 6, 2020, 21:23

Commented on timApp/static/scripts/tim/app.ts line 198

Alkuperäisessä AngularJS-koodissa on tyyliin

if (!$rootScope.$$phase) {
    scope.$apply(callback);
} else if (forceAsync) {
    scope.$evalAsync(callback);
} else {
    try {
        callback();
    } catch (error) {
        $exceptionHandler(error);
    }
}

pitäisikö vastaava olla tässäkin? (Tuo forceAsync-osuus on ainakin tarpeeton, koska se on vain blur- ja focus-eventeille.)

dezhidki commented 4 years ago

In GitLab by @Smibu on May 6, 2020, 21:23

Commented on timApp/static/scripts/tim/app.ts line 200

Olisiko handleClickAndTouch parempi nimi?

dezhidki commented 4 years ago

Joo, minä kokeilinkin tuota ekassa committissa. Aloitin sen jälkeen vaan testailemaan, että saisikohan tuota pienemmäksi.

Mutta juu, lisään tuon takaisin ihan oikeellisuuden vuoksi.

dezhidki commented 4 years ago

marked as a Work In Progress

dezhidki commented 4 years ago

Merkkaan tämän vielä WIP:ksi, sillä esimerkiksi QST:n kanssa näyttää olevan vielä hieman ongelmia.

dezhidki commented 4 years ago

changed this line in version 2 of the diff

dezhidki commented 4 years ago

changed this line in version 2 of the diff

dezhidki commented 4 years ago

added 3 commits

Compare with previous version

dezhidki commented 4 years ago

added 1 commit

Compare with previous version

dezhidki commented 4 years ago

Tätä nyt jouti säätää lisää, koska edellinen korjaustapa rikkoi QST:tä siten, että vastaustaan ei saanut muuttaa sen jälkeen, kun on valinnut jonkun vaihtoehdon (vaikka ei painanut "Save"-nappulaa).

Kokeilujen jälkeen päädyin siihen, että tätä ei ehkä yleisesti voi ratkaista niin, että toimii kaikissa selaimissa. Tuo iOS:n Safari käyttäytyy aivan eri tavalla kuin muut selaimet: iOS:n Safari näköjään haluaa, että kuunnellaan sekä touchstart että click, mutta pöytäkoneilla halutaan kuunnella vain clickia. Toisin sanoin, kun korjasin tapahtumien käsittelyä iOS:llä, sama juttu meni rikki PC:llä ja toisinpäin.

Teinkin nyt niin, että User-Agentin perusteella katsotaan, onko laite iOS (iOS:llä User-Agent pitää olla varsin luotettava ainakin sillä tasolla, että tietää sen olevan iOS) ja sen perusteella kytkeä korjaus päälle.

En nyt ole varma, onko liiankin purkkaviritelmäratkaisu ja sittenkään mergauksen arvoinen -- pidemmän päälle pitäisi kaikki komponentin vaihtaa Angulariin. Jos mikään, tämä testi oli hyödyllinen siinä mielessä, että oppi AngularJS:n rajoituksista.

Timdevsilla on nyt testisivu QST-pluginin toiminnalle, jossa uutta korjausta saa kokeilla.

https://timdevs01-2.it.jyu.fi/view/users/test-user-1/qst-test

dezhidki commented 4 years ago

In GitLab by @Smibu on May 8, 2020, 11:56

Commented on timApp/package.json line 39

Siirrä tämä alemmas kohtaan devDependencies.

dezhidki commented 4 years ago

changed this line in version 4 of the diff

dezhidki commented 4 years ago

added 1 commit

Compare with previous version

dezhidki commented 4 years ago

added 35 commits

Compare with previous version

dezhidki commented 4 years ago

added 1 commit

Compare with previous version

dezhidki commented 4 years ago

Sain nyt testattua ainakin iOS 11-13, mutta en sitä vanhemmilla.

Jostain syystä esim. iOS 8 ja vanhempi ei pidä timdevsin sertifikaatista, ja yhdellä laitteella se päästä läpi, muttei anna kirjautua (tulee se perintainen "CSRF token is missing", vaikka https:n kautta sivu lähetetään).

dezhidki commented 4 years ago

Lisätestien jälkeen eri laitteilla (ml. hieman vanhempia iOS 7 ja 8) näyttää siltä, että

Eli luotettavasti tätä ei saatukaan korjattua, ja en usko, että nyt on aikaa tätä testata jatkuvasti. Siis tähän hätään mielestäni pitää vaan todeta, että iOS:llä kannattaa käyttää Chrome-selainta tai tyytyä siihen, että kaikki pitää tuplaklikata.

No, suljen tämän MR:n toistaiseksi ja pitää katsoa, saadaanko tähän palattua.

dezhidki commented 4 years ago

closed

dezhidki commented 4 years ago

In GitLab by @vesal on May 8, 2020, 18:33

Eli luotettavasti tätä ei saatukaan korjattua, ja en usko, että nyt on aikaa tätä testata jatkuvasti. Siis tähän hätään mielestäni pitää vaan todeta, että iOS:llä kannattaa käyttää Chrome-selainta tai tyytyä siihen, että kaikki pitää tuplaklikata.

No, suljen tämän MR:n toistaiseksi ja pitää katsoa, saadaanko tähän palattua.

Siis eikös se uudemmilla toiminut korjauksen kanssa?

dezhidki commented 4 years ago

Siis eikös se uudemmilla toiminut korjauksen kanssa?

Aivan upouusilla laitteilla (alkaen iOS 11) toimii, juu. Ongelmia on nyt ns. "peruslaitteiden" kanssa (melko yleiset, mutta hieman vanhemmat) kuten iPad 4, iPad Air, iPad Minit. Käytetyt laitteet nekin ovat, mutta en kyllä saanut toimimaan kunnolla.

Jos tämä mergetaan, niin pitäisi toimea useammalla iPadilla kuin ennen (mutta mikä tahansa positiivinen lukuhan on suurempi kuin nolla), mutta ei kaikilla. Sitten ihmetellään, kun yksi henkilö ilmoittaa, että vaatii tuplaklikkauksen, sillä aikaan, kun toisella on kaikki OK.

En nyt tiedä; mielestäni on parempi, että korjaus toimisi kaikilla yleisillä iPadeilla. Kokeen kannalta on siinä mielessä parempi vaatia, että käytetään Chrome-selainta (tai vaan hyväksytään, että kaikilla iPadeilla on tuo vika -- ainakin on yhtenäisesti kaikilla sama bugi kuin valikoivasti).

dezhidki commented 4 years ago

In GitLab by @vesal on May 8, 2020, 19:26

En nyt tiedä; mielestäni on parempi, että korjaus toimisi kaikilla yleisillä iPadeilla. Kokeen kannalta on siinä mielessä parempi vaatia, että käytetään Chrome-selainta (tai vaan hyväksytään, että kaikilla iPadeilla on tuo vika -- ainakin on yhtenäisesti kaikilla sama bugi kuin valikoivasti).

Hyväksytään vika. Oliko ongelma se että korjaus ei toimi vanhoissa vai se ei niissä toimi koska ei tunistu iOSksi?

dezhidki commented 4 years ago

Oliko ongelma se että korjaus ei toimi vanhoissa vai se ei niissä toimi koska ei tunistu iOSksi?

Vähän kumpaakin. Vanhoissa ei toiminut, ja kun pyysin testaamaan timdevsia muutamalta kaverilta, joilla on uudempi laite, joillakin laitteella korjaus toimi vasta, kun otin User-Agent tarkistuksen pois (osittain; QST ei siltikään suostunut toimimaan yhdellä painalluksella).

Tähän auttaisi se, että olisi ihan oikea laite kourassa, jolloin saisi samalla säätää laitteen omia asetuksia. Muuten olen turvautunut laite-emulaattoreihin ja tuttavien etätestaamiseen (ja heitä ei oikein tohdi pyytää testailemaan sivua viiden minuutin välein :).

Tosiaankin kunnon korjaaminen vaatisi kunnon testaamista ja lisää aikaa näköjään; tällä hetkellä tämänkin korjauksen toiminta on ihan sillisalaattia.

dezhidki commented 4 years ago

In GitLab by @vesal on May 8, 2020, 19:53

Tosiaankin kunnon korjaaminen vaatisi kunnon testaamista ja lisää aikaa näköjään; tällä hetkellä tämänkin korjauksen toiminta on ihan sillisalaattia.

No jos se korjaantuu sillä A9:llä sitten kunnolla. Kunhan mun iPadillä toimii :-)

dezhidki commented 4 years ago

No jos se korjaantuu sillä A9:llä sitten kunnolla. Kunhan mun iPadillä toimii :-)

Sen voi testata:

https://timdevs01-2.it.jyu.fi/view/users/test-user-1/qst-test

dezhidki commented 4 years ago

In GitLab by @vesal on May 8, 2020, 19:55

https://timdevs01-2.it.jyu.fi/view/users/test-user-1/qst-test

Joo, siis minusta aamulla toimi kun kokeilin.

dezhidki commented 4 years ago

Joo, siis minusta aamulla toimi kun kokeilin.

Jaa, no siinä tapauksessa voi tämän puolestani mergetä, jos hyväksyy sen, että se toimii vaihtelevasti eri iPadeilla :)

Avaan nyt MR:n ja otan WIPin pois. Jos se on sinulle ja Mikalle OK, niin merge sitten

En nyt kuitenkin rupea enää ainakaan tätä viikonloppua käyttämään tämän fixaamiseksi -- etenkin, kun ainoa tapa testata itselläni on emulaattoreilla.

dezhidki commented 4 years ago

reopened

dezhidki commented 4 years ago

unmarked as a Work In Progress

dezhidki commented 4 years ago

In GitLab by @vesal on May 8, 2020, 20:05

ok

dezhidki commented 4 years ago

In GitLab by @Smibu on May 8, 2020, 20:17

Onkos tuo lisätty ua-parser-kirjasto kuinka iso? Mietin, että tätä varten ei välttämättä kannata lisätä uutta kirjastoa, jos se kasvattaa JS:n kokoa.

dezhidki commented 4 years ago

Näyttää silltä, että minimoitunakin on jossain 10 kB:n maissa.

Mutta hmmm... Tuosta saisi kyllä revittyä irti tarvittavan regexin iOS:n tunnistukselle: https://github.com/faisalman/ua-parser-js/blob/master/src/ua-parser.js#L752-L754

Minäpä koitan vaikkapa tehdä niin.

dezhidki commented 4 years ago

closed

dezhidki commented 4 years ago

reopened

dezhidki commented 4 years ago

added 26 commits

Compare with previous version

dezhidki commented 4 years ago

added 1 commit

Compare with previous version

dezhidki commented 4 years ago

added 1 commit

Compare with previous version

dezhidki commented 4 years ago

added 15 commits

Compare with previous version

dezhidki commented 4 years ago

Nyt tehty omalla tarkistuksella ilman kirjastoa.

dezhidki commented 4 years ago

Uusin (ja todnäk viimeinen) testattava versio nyt timdevsilla:

https://timdevs01-2.it.jyu.fi/view/users/test-user-1/cstest

dezhidki commented 4 years ago

added 18 commits

Compare with previous version

dezhidki commented 4 years ago

resolved all threads

dezhidki commented 4 years ago

In GitLab by @Smibu on May 11, 2020, 13:38

Hmm... mun iPadilla (13.3) ajopainikkeiden painelu vuorotellen ei itse asiassa toimi kun nyt kävin kokeilemassa tuolla: https://timdevs01-2.it.jyu.fi/view/users/test-user-1/cstest

Eli joudun painamaan aina kahdesti.

(Sitä edellistä ua-parser-versiota en tullut testanneeksi.)

Tsekkasitko Denis, että sulla kuitenkin toimii vielä?

dezhidki commented 4 years ago

Tsekkasitko Denis, että sulla kuitenkin toimii vielä?

Juu, näyttää tomivan samalla tavalla Browserstackissa (tosin uusin laite, jolla siellä voi etätestata on iPad 12.9).

Miten sitten @vesal tabletilla? Toimiiko https://timdevs01-2.it.jyu.fi/view/users/test-user-1/cstest edelleen?

Hmm... mun iPadilla (13.3) ajopainikkeiden painelu vuorotellen ei itse asiassa toimi

Joo, tämäkään korjaus ei näytä toimivan täydellisesti kaikilla pädeillä (vaikka user-agent tarkistuksen ottaisi pois). Tämän takia vähän ehdotinkin aiemmin, että pitäisikö ehkä laittaa tämä MR toistaiseksi kiinni -- en usko, että ehdin tätä korjaamaan täydellisesti ennen kokeita.

dezhidki commented 4 years ago

In GitLab by @Smibu on May 11, 2020, 14:07

Okei joo, no jos Vesan tabletilla tuo viimeisin versio toimii, niin mergetään vain. Angulariin siirtyminen ennen pitkää on tosiaan sitten se kunnon korjaus.

dezhidki commented 4 years ago

added 15 commits

Compare with previous version

dezhidki commented 4 years ago

added 16 commits

Compare with previous version