Repository navigation
Conversation
EreMaijala
left a comment
EreMaijala
left a comment
There was a problem hiding this comment.
- Pidempiä tekstejä ei pidä koota yhdistelemällä erilaisia palasia. Siitä tulee käännöspainajainen. Pitää lisätä uusia käännöksiä tarpeen mukaan erilaisille kokonaisuuksille.
- Miksi suljetut ja "ei tietoa" on samassa listassa?
- Eikö olisi selkeämpää listata toimipisteet aakkosjärjestyksessä ja jokaisen kohdalla tieto aukiolosta? Nyt tiettyä kirjastoa etsivä voi joutua käymään läpi kaksi listaa.
- Onko tälle listalle todellinen käyttötarve ainakaan tällaisenaan? Pitäisi päästä vähintään tarkempiin tietoihin kunkin kirjaston kohdalta, koska pelkkä tieto tämänhetkisestä aukiolosta ei kerro kovin paljon, ja kartaltakin klikkaamalla saa toimipisteen tiedot esille.
|
Nyt on rakennettu systeemi uusiksi. |
|
Tarkistan vielä yhden jutun... |
EreMaijala
left a comment
EreMaijala
left a comment
There was a problem hiding this comment.
Tuo kolmetasoinen luuppi on edelleen tosi sekava ja johtaa siihen, että kaikki pitää käsitellä kolmeen kertaan. Käy mieluummin kerran läpi ja jaa kaikki kolmeen eri taulukkoon. Sitten niiden näyttäminen yksi kerrallaan on selkeää.
|
|
||
| map = finna.organisationMap; | ||
| map.init(mapWidget, mapTileUrl, mapAttributionTemplate.innerHTML); | ||
| mapWidget.setAttribute('aria-hidden', 'true'); // prevent screenreaders to accientally enter map area |
There was a problem hiding this comment.
Miksei tätä ole asetettu suoraan .phtml-tiedostossa? Sama kysymys kaikkien alempana olevien kohdalla.
There was a problem hiding this comment.
Tämä edelleen. Kyllä sen pitää toimia niin, että attribuutti on suoraan HTML:ssä valmiiksi. Muutenhan ei mikään vastaava toimisi missään.
EreMaijala
left a comment
EreMaijala
left a comment
There was a problem hiding this comment.
- location-selection.phtml:ään on ilmestynyt melkoinen määrä whitespace-muutoksia, joilla ei ole mitään tekemistä tämän taskin kanssa. Pois sellaiset. Jos jotain isompaa yhtenäistämistä tai siivoamista on, niin erillisena pull requestina sellaiset, kun eivät vaikuta tänä toiminnallisuuteen.
- Jos name on tyhjä, niin jätä pois jo listojen kokoamisvaiheessa, niin ei tarvitse tarkistella enää kun listat tulostetaan.
- Eikö tuon listan pitänyt olla visually hidden? Nyt se on aikamoinen taulu. Neljä palstaa on liikaa mobiilissa, ja koko lista on väärässä kohdassa toimipaikan perustietojen ja lisätietojen välissä.
| color: $gray-dark; | ||
| } | ||
|
|
||
| .column-list { |
There was a problem hiding this comment.
Liian geneerinen luokka ollakseen ylätasolla määriteltynä näin. Oliko tarkoitus laittaa .organisation-info:n sisään? Silloinkin sillä voisi olla jokin käyttötarkoitusta kuvaavampi luokan nimi.
| $nowOpen = $this->transEsc('organisation_info_is_open_now'); | ||
| $nowClosed = $this->transEsc('organisation_info_is_closed_now'); | ||
| $nowMobile = $this->transEsc('organisation_info_no_schedule'); |
There was a problem hiding this comment.
Turhat muuttujat, kun näitä käytetään vain yhdessä kohdassa.
| <h3><?= $this->transEsc('Location Service') ?></h3> | ||
| <div class="visually-hidden"> <?= $this->transEsc('organisation_info_navi_service') . ' ' . $this->transEsc('Total') . ' ' . $locationCount ?></div> | ||
| <?php if ($locationCount > 1): ?> | ||
| <h4><?= htmlspecialchars($this->escapeHtml($nowOpen)); ?></h4> |
There was a problem hiding this comment.
Nyt on tripla-escape näillä, kun muuttujassakin on escapettu arvo.
| } | ||
| ?> <!-- display categorised organisations --> | ||
| <h3><?= $this->transEsc('Location Service') ?></h3> | ||
| <div class="visually-hidden"> <?= $this->transEsc('organisation_info_navi_service') . ' ' . $this->transEsc('Total') . ' ' . $locationCount ?></div> |
There was a problem hiding this comment.
| <div class="visually-hidden"> <?= $this->transEsc('organisation_info_navi_service') . ' ' . $this->transEsc('Total') . ' ' . $locationCount ?></div> | |
| <div class="visually-hidden"> | |
| <?=$this->transEsc('organisation_info_navi_service')?> <?=$this->transEsc('Total')?> <?=$locationCount?> | |
| </div> |
| <div class="column-list"> | ||
| <?php foreach ($openList as $item): ?> | ||
| <?php if ($item['name'] != ''): ?> | ||
| <div><a href="#<?= $this->escapeHtmlAttr($item['id']) ?>"><?= $this->escapeHtml($item['name']) ?></a></div> |
There was a problem hiding this comment.
Onko tuolla ympäröivällä div:llä jokin merkitys tässä?
| <?php endif; ?> | ||
| </div> | ||
| <!-- END of: finna - organisationinfo/elements/location-selection.phtml --> | ||
| <!-- END of: finna - organisationinfo/elements/location-selection.phtml --> No newline at end of file |
| } | ||
| } | ||
| ?> <!-- display categorised organisations --> | ||
| <h3><?= $this->transEsc('Location Service') ?></h3> |
There was a problem hiding this comment.
Väärä otsikko (ks. käytetty käännös).
|
Nyt koko systeemin pitäisi toimia fokussiirtoineen päivineen. Pasin ehdotuksesta siirrettiin tuo - Näytä kaikki palvelupisteet - pois kartan alueelta, jotta myös ruudunlukijat pääsevät helposti selailemaan uudelleen koko listaa jos ovat valinneet jonkun kirjaston ja haluavat sen jälkeen tarkastella toisen kirjaston tietoja. Fokus siirto toimii automaattisesti kun valitsee listalta kaikilla selaimilla ja ruudunlukijioilla JAWS:ia lukuunottamatta, joka jostain syystä haluaa aina siirtyä sivun alkuun toistaen kuitenkin oikean palvelupisteen valinnan, jolloin käyttäjä joutuu hyppäämään pari otsikkoa alemmaksi päästäkseen samoihin tietoihin mihin muilla ruudunlukijoilla pääsee suoraan. |
|
Käytiin tätä läpi tänään UX:ssä, teen tuohon hieman muutoksia vielä. |
EreMaijala
left a comment
EreMaijala
left a comment
There was a problem hiding this comment.
Muutoksissa on edelleen suuri määrä tähän tehtävään kuulumattomia välilyönti- ja rivinvaihtojen säätöjä.
|
Juu, pahoittelut noista turhista tyhjien rivien poistoista ja ylimääräisistä tyhjistä väleistä. Yritän olla tarkempi vastaisuudessa. Tuo oli ensimmäinen iso JS tiedoston editointi, seuraavalla kerralla sitten paremmin ; ) |
EreMaijala
left a comment
EreMaijala
left a comment
There was a problem hiding this comment.
Ylimääräisiä muutoksia on edelleen ainakin location-selection.phtml:ssä.
This will create a sorted list for the screenreaders describing which libraries are open and closed or miss the proper information. Tested on all screenreaders and browsers.