Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Directory listing access/visibility level badges #3680

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

saviit
Copy link
Contributor

@saviit saviit commented Jul 17, 2024

Resolves #3679

Replace lock icon for directory list items with badges indicating access level. Also adds info column for document tags.

image


Badge texts/ color scheme (currently using colors from the official JYU color scheme):

Public (#002957): Item is visible publicly (ie. to everyone), including anonymous users.
Logged-in (#6b7f97): Item is visible to logged-in users.
Organization (#c29a5b): Item is visible to groups belonging to a Haka organization (jyu.fi users, helsinki.fi users, etc. etc.).
Limited (#ea9d90): Item is visible only to specific users, check the Manage-page for details.
Private (#f1563f): Item is visible only to its owners.

TODO:

  • proper scheme for badges
    • what badges are needed
    • how do badges map to different access right combinations
  • consistent styling according to UI theme used, mainly coloring
  • get access info for all directory items in one http request instead of doing a request for each item
  • show document tags as badges as well (slightly different style from access badges)
  • localize badge texts
  • user setting to toggle new directory list badges on and off? what should be the default setting?

@saviit saviit force-pushed the feature-dir-list-badges branch from 0896862 to 736f18b Compare August 20, 2024 08:39
timApp/item/routes.py Outdated Show resolved Hide resolved
@dezhidki
Copy link
Member

Tämän voisi ihan alkuun laittaa johonkin timbetan testipalvelimelle (ks. sähköposti).

Nopeasti katsottuna pääosin OK. Oliko joku syy, miksi noita items-muuttujassa olevaa dataa ei käytetä oikeuksien tarkasteluun?

<script>
var items = {{ items | tojson }};
</script>

Tuo lista tulee tuosta tuosta:

TIM/timApp/item/routes.py

Lines 1213 to 1221 in 8b5cf09

def get_items(folder: str, recurse=False):
u = get_current_user_object()
docs = get_documents(
search_recursively=recurse, filter_folder=folder, filter_user=u
)
docs.sort(key=lambda d: d.title.lower())
folders = Folder.get_all_in_path(root_path=folder, recurse=recurse)
folders.sort(key=lambda d: d.title.lower())
return [f for f in folders if u.has_view_access(f)] + docs

Joka muunnetaan JSONiksi. Silloin tullaan kutsumaan Item.to_json, jossa on esimerkiksi tuo unpublished-merkki:

TIM/timApp/item/item.py

Lines 165 to 186 in d5136e5

def to_json(self, curr_user: User | None = None):
if curr_user is None:
from timApp.auth.sessioninfo import get_current_user_object
curr_user = get_current_user_object()
return {
"name": self.short_name,
"path": self.path,
"title": self.title,
"location": self.location,
"id": self.id,
"modified": date_to_relative(self.last_modified)
if self.last_modified
else None,
"owners": self.owners,
"rights": get_user_rights_for_item(self, curr_user),
"unpublished": self.block.is_unpublished() if self.block else False,
"public": self.public,
# We only add tags if they've already been loaded.
**include_if_loaded("tags", self.block),
**include_if_loaded("relevance", self.block),
}

Siinä tulee esim valmiiksi jo nykyisen käyttäjän oikeudet rights-muuttujassa. Tagit eivät oletuksella tule, mutta saahan niitäkin. Jos haluaa ryhmien nimet, niin niitäkin voi tuohon jsoniin saada lisättyä. Tai voisi suoraan palvelimen puolella laskea, mitä "oikeustageja" pitäisi laittaa.

Tuo lista pitäisi olla valmiiksi käytössä this.itemList-muttujassa siellä DirectoryListComponentissa. Pääosin hyöty olisi, ettei tarvitsisi tehdä useita HTTP-kutsuja, jotta ei tule esim. hitaalla netillä ylimääräistä elementtien ilmestymistä.

@saviit
Copy link
Contributor Author

saviit commented Aug 23, 2024

Tämän voisi ihan alkuun laittaa johonkin timbetan testipalvelimelle (ks. sähköposti).

Nyt löytyy tuolta tim01, en tosin vielä ehtinyt tehdä testihakemistoa/dokumentteja.
Testihakemisto timbeta01:ssa
Haka-ryhmiä tosin ei ole tuolla testipalvelimella, niin se Organisation-lätkä ei näy.

Oliko joku syy, miksi noita items-muuttujassa olevaa dataa ei käytetä oikeuksien tarkasteluun?

Joo, nuo mitä tuolta saa irti ei riitä siihen mitä tässä haluttiin tehdä. Tässähän haluttiin ymmärtääkseni, että se badge kertoo, mille käyttäjäryhmille/missä laajuudessa mikäkin dokumentti tai kansio on näkyvissä. Tuosta Item.to_jsonista saa irti vain sen, mitkä ovat nykyisen käyttäjän oikeudet kys. kohteeseen.

@dezhidki
Copy link
Member

dezhidki commented Aug 23, 2024

Tuosta Item.to_jsonista saa irti vain sen, mitkä ovat nykyisen käyttäjän oikeudet kys. kohteeseen.

Niin, tarkoitan juuri siis, että mielestäni tuota Item.to_jsonia voi laajentaa lisäämällä sihen lisäattribuuttina esim. visibility, joka laskettaisiin valmiiksi palvelimella samalla logiikalla kuin mitä tehdään nyt JS:ssä. Tällöin ei tarvitse tehdä aina HTTP-kutsuja.
Ehkä mun pointin näkee selkeämmin kun esimerkiksi käyttäjällä on hitaampi netti:

bookmark.mp4

Tuo on vielä OK kahdella dokumentilla, mutta jos on Ohj1 tasoinen kansiorakenne, niin noiden kaikkien eri versioiden hyppiminen on vähän ylimääräistä IMO.

Ylläpidon kannalta tuon Item.to_jsonin laajentamine olisi myös järkevää, sillä nykyinenkin unpublished-merkki (josta tuli se lukon ikoni) oli valmiiksi laskettu palvelimelta. Lisäksi tuo uusi /items/accesses/<int:item_id>-polku tarkoittaa, että kuka tahansa view-oikeuden omaava voi nyt saada tiedon dokumentin kaikista oikeuksista. En heti osaa sanoa, onko tuosta haittaa, mutta kyllähän tuosta käyttäjälle paljastuu enemmän tietoa kuin mitä tarvitsisi.

Nojoo, ei tuo ole mielestäni mikään merkittävä asia, mikä estäisi mitään, ehkä makukysymys. Ehkä korkeintaan vielä voisi pohtia, onko perää lähettää kaikki oikeudet tuossa /items/accesses/<int:item_id>-polussa, vai olisiko parempi palauttaa vain tagin arvon (Public, Logged-in, Limited, Private) ja tarvittaessa selitteen.

@saviit
Copy link
Contributor Author

saviit commented Aug 23, 2024

Niin, tarkoitan juuri siis, että mielestäni tuota Item.to_jsonia voi laajentaa lisäämällä sihen lisäattribuuttina esim. visibility

Aivan, lukaisin tuon aikaisemman vähän hätäisesti. Joo, tuo kuulostaa fiksulta, eipä käynyt aikaisemmin mielessä.

Lisäksi tuo uusi /items/accesses/<int:item_id>-polku tarkoittaa, että kuka tahansa view-oikeuden omaava voi nyt saada tiedon dokumentin kaikista oikeuksista.

onko perää lähettää kaikki oikeudet tuossa /items/accesses/<int:item_id>-polussa, vai olisiko parempi palauttaa vain tagin arvon (Public, Logged-in, Limited, Private) ja tarvittaessa selitteen.

Joo, tässä ei ollut vielä noin pitkälle mietitty, kunhan jotain saa näytölle että voi testata mikä on hyvä tapa visualisoida noita (+ onko tarvetta vielä muille elementeille). Lähden seuraavaksi kehittämään tuohon suuntaan että minimoidaan kutsujen ja datan määrää.

@dezhidki
Copy link
Member

Testasin timbeta01:ssä, hyvin vaikuttaa toimivan 🙂.

Minusta näkyvyystasoja merkitsevien tagien ulkoasua on syytä vielä pohtia hieman. Tällä hetkellä tagit ovat "liian vahvasti" näkyvissä, jolloin huomio kohdistuu niihin liikaa. Testasin siis seuraavaa sivua: https://timbeta01.tim.education/view/users/test-user-1

Minua pistää silmään ehkä seuraavat asiat:

  • Tagien värit ovat liian räikeät muun käyttöliittymän suhteen. Erityisesti Public on ehkä vähän liian vahvasti näkyvissä, vaikka tajuankin kyllä pointin.
  • Sen sijaan Private-tagin kontrasti on liian matala taustan värin suhteen, jolloin sitä on vaikeaa erotella.
  • Tagi on itsessään suurempi kuin dokumentin/kansion nimi, jolloin se vie huomion.
  • Tämän lisäksi tagien teksti on kursivoitu, mikä sekin kiinnittää turhaa huomiota.

Minusta näkyvyystagin pointti on pääosin vain helpottaa näkyvyyden tunnistamisen ilman, että pitäisi mennä manageen. Vertaa esimerkiksi siihen, miten GitHubissa näkyvyystagit on toteutettu:

image

Siinä mielessä esimerkiksi joku tuollainen tyyli voisi olla käypä:

image

Tai ehkä boldilla?

image

Silloin tosin fonteista tulee taas epätasapainoisia.

Olisiko perää laittaa nämä kaikki näkyvyystagit jopa omaan sarakkeeseen tässä listauksessa? Ihan vaan jotta ne olisivat aina samalla tasolla?

Jos värejä haluaa, niin minusta niistä pitäsi keksiä vähän hillitympiä versioita, jotta ne eivät vie liikaa huomiota. Esimerkiksi:

image

Värejä valitessa voinee pyrkiä siihen, että ne ovat suunnilleen yhtä kirkkaat.

Tämä nyt vaan sellaista ajatusvirtaa, mielipiteitä saa heittää 😁

@saviit
Copy link
Contributor Author

saviit commented Aug 23, 2024

Tagien värit ovat liian räikeät muun käyttöliittymän suhteen.

Saatoin laittaa tarkoituksella maximum overloadilla että varmasti tulee keskustelua ;)

Siinä mielessä esimerkiksi joku tuollainen tyyli voisi olla käypä:

Kumpainenkin näyttää fiksulta, boldaus on mielestäni toisaalta myös perusteltu erottuvuuden puolesta, mutta ehkäpä nuo jo muutenkin erottuvat tarpeeksi.

Olisiko perää laittaa nämä kaikki näkyvyystagit jopa omaan sarakkeeseen tässä listauksessa? Ihan vaan jotta ne olisivat aina samalla tasolla?

Tämä oli itselläkin mielessä, ja jos nuo dokumenttitägit jätetään näkyviin niin mielestäni molemmille silloin tarvitaan omat sarakkeet. Samalla sitten vähän yksinkertaistuu käyttäjäasetuksen tekeminen, millä nuo saa kytkettyä päälle ja pois.

@dezhidki
Copy link
Member

Kokeillaan vaikka alkuun ilman boldausta ja vähän hillitymmillä väreillä (kuitenkin kontrastia pitää olla jotta erottuu taustasta). Jos ne ovat vielä omassa sarakkeessa, niin sitten dokumenttilistaus saa selkeämmäksi.

Jos haluaa käyttää voimakkaampia värejä, niin sitten mieluummin sellaiset, jotka sopivat nykyiseen värimaailmaan (sinistä, oranssia + niiden vastavärejä). Mutta kokeilulla selviää.

@vesal
Copy link
Contributor

vesal commented Aug 24, 2024 via email

@saviit saviit force-pushed the feature-dir-list-badges branch from b955728 to ca48263 Compare October 14, 2024 10:12
@saviit saviit force-pushed the feature-dir-list-badges branch 2 times, most recently from 3386391 to f020179 Compare December 19, 2024 09:19
@saviit saviit force-pushed the feature-dir-list-badges branch 2 times, most recently from 696fac6 to 5dd8de9 Compare January 3, 2025 10:41
@saviit saviit force-pushed the feature-dir-list-badges branch from 5dd8de9 to 655da14 Compare January 9, 2025 08:34
@saviit
Copy link
Contributor Author

saviit commented Jan 9, 2025

Tämä olisi nyt omasta puolestani valmis, testata voi timbeta03:ssa esim.
https://timbeta03.tim.education/view/users/test-user-1/dir-list-badges

Värit on valittu toistaiseksi JYU:n virallisesta väripaletista, jos haluaa vähän rauhallisemman niin voi testata laittamalla TIM settingsin custom CSS lootaan vaikkapa seuraavat:

.ab-public {
    border: solid 2px #686868 !important;
    color: #686868 !important;
}
.ab-logged-in {
    border: solid 2px #a2a2a2 !important;
    color: #a2a2a2 !important;
}
.ab-organization {
    border: solid 2px #a2a2a2 !important;
    color: #a2a2a2 !important;
}
.ab-limited {
    border: solid 2px #a2a2a2 !important;
    color: #a2a2a2 !important;
}
.ab-private {
    border: solid 2px #a2a2a2 !important;
    color: #a2a2a2 !important;
}

@saviit saviit requested review from dezhidki and sijualle January 9, 2025 14:56
@dezhidki
Copy link
Member

Ulkonäöllisesti on aika hyvä. Ehkä vieläkin ehdotan sitä, että labeleiden sisällä oleva teksti ei olisi kursivoitu. Tällä hetkellä esim. tuolla mallisivullasi labeleiden teksti on ainoa kursivotu, mikä vie vähän turhaa huomiota. Värit toimivat jo hyvin huomion kiinnittäjänä.

Vastaavasti, kun vie kursorin päälle, niin niihin tulee alleviivaus:

image

Minusta sitä ei tässä tarvitse, vaan noista labeleista voisi ehkä ottaa -elementti pois ellei se tarvitaan johonkin. Tai ainakin voisi laittaa pointer-events: none.

Labeleihin voisi laittaa vielä white-space: nowrap, sillä nyt kapeammalla näytöllä labelit menevät oudosti moniriviseksi:

image

Tai sitten pitäisi tehdä vähän muutosta elementteihin, jotta labelin sisällä oleva teksti rivittyisi, muttei label itse.

Koitan katselmoida koodiakin piakkoin, tässä alkuun siis ulkonäköön liittyviä juttuja.

@saviit
Copy link
Contributor Author

saviit commented Jan 13, 2025

Ulkonäöllisesti on aika hyvä. Ehkä vieläkin ehdotan sitä, että labeleiden sisällä oleva teksti ei olisi kursivoitu. Tällä hetkellä esim. tuolla mallisivullasi labeleiden teksti on ainoa kursivotu, mikä vie vähän turhaa huomiota. Värit toimivat jo hyvin huomion kiinnittäjänä.

Vastaavasti, kun vie kursorin päälle, niin niihin tulee alleviivaus:

Minusta sitä ei tässä tarvitse, vaan noista labeleista voisi ehkä ottaa -elementti pois ellei se tarvitaan johonkin. Tai ainakin voisi laittaa pointer-events: none.

Labeleihin voisi laittaa vielä white-space: nowrap, sillä nyt kapeammalla näytöllä labelit menevät oudosti moniriviseksi:

Joo, näistä taisikin olla aikaisemmin puhetta, mutta pääsi unohtumaan. Korjailen nämä kohta.

EDIT: nämä nyt päivitetty timbeta03:een
Tagien labelit on nyt inline-flexissa wrapattuna, nopealla kokeilulla rivittyvät nyt nätisti.

@sijualle
Copy link
Contributor

Testailin toisella käyttäjällä ja kesti hetki tajuta että pitää käydä ensin laittamassa asetuksista badget näkyviin. Voisi johonkin jatkoon miettiä saisiko jotenkin kivasti integroitua tuohon kansionäkymään suoraan jotain painiketta jolla nuo saisi näkyviin (vai tekisiköhän se kansionäkymästä vain sekavamman)

@saviit
Copy link
Contributor Author

saviit commented Jan 13, 2025

Testailin toisella käyttäjällä ja kesti hetki tajuta että pitää käydä ensin laittamassa asetuksista badget näkyviin. Voisi johonkin jatkoon miettiä saisiko jotenkin kivasti integroitua tuohon kansionäkymään suoraan jotain painiketta jolla nuo saisi näkyviin (vai tekisiköhän se kansionäkymästä vain sekavamman)

Joo, laittelin tuon oletusasetuksen falseksi, koska tavllisella käyttäjällä ei pitäisi juurikaan olla tarvetta tälle ominaisuudelle. Toinen vaihtoehto voisi olla, että nuo on oletuksena päällä, mutta sitten siellä listauksen puolella nuo Access ja Tags sarakkeet olisi oletuksena 'puolipiilossa', eli pitäisi klikata otsakkeita 'A' ja 'T' että ne tulee näkyviin. Siinä sitten puolestaan on ongelmana se, että ominaisuutta tarvitseville tuo olisi aina ylimääräinen askel, eli varmaan aiheuttaisi enemmän ärsytystä...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parempi merkintä dokumentin julkisuustasosta kansion dokumenttilistaukseen
5 participants