MultiselectFilter używający select z parametrem multiple

[michauciak]

Dodany zostaje filtr MultiselectFilter, który używa znacznika HTML select z parametrem multiple. Użyty jest dla rodzaju przedmiotu i opiekuna przedmiotu. Zapewne wypadałoby też go użyć w ofercie przedmiotów w analogicznych miejscach.

Na komputerze wybieranie wielu pozycji z listy przebiega z użyciem klawisza Ctrl. Wybranie pierwszej wartości na liście, czyli w zasadzie tytułu listy, powoduje wyzerowanie wszelkich zaznaczeń na liście. Wybranie dowolnej pozycji na liście bez klawisza Ctrl powoduje zwyczajny, pojedynczy wybór.

Closes #746.

[lgpawel]

Chyba całkiem sprawnie to działa, ale za to trochę topornie wygląda i warto trochę przeorganizować ten panel z filtrami. Mój wstępny pomysł (przy którym oczywiście się nie upieram i chętnie zobaczę lepsze propozycje) byłby taki:

1. Zrobić układ w dwóch wierszach (zamiast w trzech kolumnach, jak obecnie).
2. Pierwszy wiersz zawierałby od prawej dwa multiselektory tak jak teraz, każdy na szerokości nieco mniejszej niż obecna ⅓ (treści, które się w nich pojawiają, nie są przesadnie długie), a na reszcie, pod sobą nazwa przedmiotu i ptaszek zalecanych tylko dla pierwszaków (jak będzie więcej miejsca, to napis nie będzie się zawijał – przynajmniej u mnie 😏). Trochę jeszcze nie wiem, co zrobić z przyciskiem czyszczenia filtrów (który, w razie potrzeby, mógłby być wielolinijkowy i zawierać słowo "wszystkie").
3. W drugim wierszu chmury tagów, z miejscem podzielonym oczywiście po równo.

Co do szczegółów działania i wyglądu multiselektorów: wydaje mi się, że pierwsza pozycja o treści "-- [nazwa własności] --" słabo się broni. Dałbym nagłówek (tak, jak przy chmurach tagów), a pierwszą pozycję nazwał odpowiednio "[Wszystkie]" bądź "[Wszyscy]", można by ją też ostylować na jakąś szarą kursywę czy coś. I to, że ona może być zaznaczona, zwłaszcza razem z innymi, jest dość nielogiczne – efekt kliknięcia na nią jest sensowny taki, jaki jest, ale moim zdaniem powinna być niezaznaczalna.

BTW przeszła mi przez głowę trochę niedobra na tym etapie myśl: a czym to się różni od chmury tagów? (Oprócz tego, że czyszczenie pojedynczego filtra jest wygodniejsze…) Oczywiście opiekunów jest za dużo, żeby ich prezentować w postaci chmury, ale rodzaje przedmiotów? Sam nie mam zdania, bardzo rzadko tych filtrów używam.

[michauciak]

Wydaje się, że zasadniczo powyższe uwagi zdołałem uwzględnić:

[michauciak]

BTW przeszła mi przez głowę trochę niedobra na tym etapie myśl: a czym to się różni od chmury tagów? (Oprócz tego, że czyszczenie pojedynczego filtra jest wygodniejsze…) Oczywiście opiekunów jest za dużo, żeby ich prezentować w postaci chmury, ale rodzaje przedmiotów? Sam nie mam zdania, bardzo rzadko tych filtrów używam.

Tak, też tę myśl mam. Może wystawię jeszcze jedną wersję – z użyciem chmury tagów do jedynie rodzaju przedmiotów. I chyba na tym bym zakończył wtedy zajmowanie się tym issue, wszak wciąż mam poczucie, że każde z tych podejść nie ulepsza, a raczej utrudnia korzystanie z interfejsu w większości sytuacji.

[lgpawel]

Nie podzielam sceptycyzmu. Możliwości, już teraz, są nie gorsze niż wcześniej – pojedynczym kliknięciem, po ew. jakimś przescrollowaniu, możemy wybrać pojedynczą grupę. Zajmuje to trochę więcej miejsca, ale to nieuniknione, poza tym zazwyczaj panel filtrów mamy i tak częściowo zwinięty.

Czymś, co odróżnia tagi i efekty od kategorii i właścicieli, co być może jakoś podświadomie uzasadnia inną prezentację, to to, że te pierwsze są z przedmiotami w związku wiele-do-wielu, a te drugie – określone dla nich jednoznacznie. No i są "ważniejsze" 😉

Natomiast otrzymany efekt wizualnie dalej mnie nie przekonuje (mimo że oczywiście wiernie odwzorowuje to, o czym pisałem wcześniej). Proszę poeksperymentować samemu; ja w którymś momencie jeszcze wrócę ze swoją opinią i ew. innymi sugestiami.

[michauciak]

Może tak jest odrobinę mniej chaotycznie:

[lgpawel]

No jest, ale ma też inne wady. Ale już wiem, jak będzie "najlepiej" (mam nadzieję). Jak sam Pan wyżej zauważył, te multiselecty przydałyby się też gdzie indziej, mianowicie w głosowaniu (gdzie dodatkowo jest jeszcze kontrolka semestru, za to z jakiegoś powodu nie ma przycisku "wyczyść") i ofercie (gdzie dochodzi jeszcze status propozycji). A że te układy powinny być do siebie możliwie podobne, to dla listy przedmiotów czy też prototypu planu i tak musimy zostawić wolne miejsce.

Wrócmy więc do układu trzech kolumn, ale chmury tagów będą w środkowej i prawej. W lewej kolejno: nazwa (jak wcześniej), semestr (dla głosowania i oferty), status (dla oferty), ptaszek dla pierwszaków oraz przycisk czyszczenia. Semestr i status też powinny być oczywiście multiselectami. Myślę, że w ich przypadku można dać pełne trzy wiersze i bez pseudopozycji "wszystkie". Docelowo dla nich można by pomyśleć o czymś oszczędniejszym objętościowo, ale to wizja na inny PR raczej.

[michauciak]

A zatem

[lgpawel]

OK, ale musimy wprowadzić jakieś sterowanie wysokością kontrolki – dla rodzaju i opiekuna lepiej dalej nie zmniejszać, dla semestru i statusu – warto. Proszę zrobić jeszcze wariant dla głosowania (czy tego nie da się jakoś wyabstrahować swoją drogą?) i sprawdzić czy nie ma niepotrzebnych importów (a chyba są, bo nagle w ogóle nie mamy zwykłych selectów).

[michauciak]

Skrócony select dla semestru i statusu:

[michauciak]

w głosowaniu (gdzie dodatkowo jest jeszcze kontrolka semestru, za to z jakiegoś powodu nie ma przycisku "wyczyść")

Proszę zrobić jeszcze wariant dla głosowania (czy tego nie da się jakoś wyabstrahować swoją drogą?)

Nawet zabawne się okazuje, że pomimo iż te pliki CourseFilter.vue wydają się tak do siebie podobne, to dodanie

  methods: {
    ...mapMutations("filters", ["clearFilters"]),
  },

w przypadku głosowania powoduje, że w ogóle się nie wyświetlają filtry. Jest tam za to

  computed: {
    ...mapGetters("filters", {
      tester: "visible",
    }),
  },

których nie ma w dwóch pozostałych plikach CourseFilter.vue. Zatem zapewne najpierw warto by przeanalizować to zachowanie.

[lgpawel]

Pierwsze wrażenie działania bardzo dobre! Na szybko mam tylko jedną poprawkę do usability: filtry, które modyfikujemy, są zawsze na górze panelu filtrów, i przez to widoczne również, kiedy ten panel jest zwinięty. Dobrze byłoby, żeby były również używalne w tej sytuacji, ale niestety rozwinięta lista opcji danego "chowa się" pod "zwinięciem" panelu – co ważne, inaczej niż w przypadku obecnych dropdownów pojedynczego wyboru. Warto to dopracować.

Z kolei w kwestii czepialstwa estetycznego można by nieco zwiększyć stopień pisma, jakim wyświetlane są opcje w filtrach (mógłby być taki sam, jak dla placeholderów "Wszystkie rodzaje" / "Wszyscy opiekunowie"), a trójkątne ikonki w przyciskach wydają się umieszczone nieco poniżej środka. Sądzę też, że przycisk z "iksem" do czyszczenia stanu filtra mógłby mieć jakąś ramkę i/lub tło (np. takie same jak ten z trójkątem), żeby było widać, gdzie on właściwie jest, ale tu jestem bardziej skłonny zmienić zdanie.

EDIT: układ panelu filtrów w liście przedmiotów został zmieniony zgodnie z dotychczasową dyskusją, ale ona była toczona w innym kontekście, tj. kiedy filtry nowego rodzaju miały mieć wielolinijkowe kontrolki. Udało się je zrobić z kontrolkami jednolinijkowymi, więc nie ma tego powodu zmieniać układu, do którego użytkownicy są już przyzwyczajeni – przywróćmy stary.

[bkaszowski]

Przywróciłem poprzedni układ i wrzuciłem poprawkę związaną ze zwijaniem panelu z filtrami.

[bkaszowski]

Drobne podsumowanie prac:

PR dodaje komponent MultiSelectFilter, który jak nazwa wskazuje pozwala na wybranie wielu filtrów na raz. Komponent jest wrapperem dla komponentu z biblioteki https://vue-multiselect.js.org/, zatem można korzystać z wszystkich funkcjonalności przez nią dostarczanych.

W ramach prac zostały dość mocno zmodyfikowane style dostarczane przez bibliotekę, zatem w celu ułatwienia modyfikacji komponentu, zostały style dodane bezpośrednio do kodu źródłowego i bezpośrednio zmodyfikowane.

Z racji wymogu, że dropdown komponentu musiał być widoczny poza “zwiniętymi” filtrami na stronie, właściwość position wrappera .multiselect_content-wrapper musiał zostać zmieniony z absolute na fixed, w związku z czym dropdown komponentu nie przyjmował szerokości rodzica tylko całego okienka i był zbyt szeroki. Jako rozwiązanie powstała funkcja updateDropdownWidth, która dostosowuje szerokość dropdownu na podstawie szerokości wrappera po stronie JS, a nie CSS.

[Telpenarmo]

Ta zmiana, i to do tej „lepszej” wersji, okazała się bardzo łatwa. Efekt jest rzeczywiście intuicyjny: napis zależy teraz od tylko wyboru elementów, nie od tego, czy menu jest otwarte.

[lgpawel]

Pozwoliłem sobie wyedytować wszystkie trzy pliki CourseFilter.vue tak, by były jak najbardziej podobne do siebie. Oczywiście jest to docelowo z myślą o tym, żeby być może je jakoś zdeduplikować, ale to na pewno nie w ramach tego PRa. Proszę o sprawdzenie, najlepiej także eksperymentalne, czy zgodnie z intencją nie zmieniam nic w działaniu stron.

Dobrze, że najświeższa zmiana okazała się nieskomplikowana, natomiast widzę teraz potrzebę jeszcze jednej poprawki: zauważyłem, że przy powybieranych opcjach stan opisującego napisu odzwierciedla kolejność zaznaczania, co jest oczywiście dość idiotyczne. Fajnie byłoby to jakoś normalizować, być może sortując listę selected? Najlepsza byłaby oczywiście dokładnie taka kolejność, jak na liście (wszystkich) opcji.

[Telpenarmo]

Dopisałem tę zmianę, chociaż rozwiązanie, które proponuję, nie jest najprostsze.
Niestety, kolejność elementów w menu nie wynika z ich wartości (etykiet ani identyfikatorów), więc zamiast sortować listę zaznaczeń należy sprawdzać kolejność ich występowania w liście wszystkich opcji – o ile, oczywiście, zależy nam na zachowaniu bieżącej kolejności elementów menu, co wydaje mi się pożądane.

Opis oryginalnej implementacji

Zaimplementowałem to też zresztą niedoskonale, używając jako funkcji porównującej liniowego przeszukania pełnej listy. Uznałem, że taka implementacja jest najprostsza, a w przypadku tak małych list inkrementacja wykładnika złożoności nie boli, ale jeśli Pan, lub kto inny, się nie zgadza, mogę też oczywiście zaimplementować coś bardziej złożonego.

Edit: niepotrzebnie się spieszyłem. Wystarczyło pomyśleć o tym jeszcze raz, żeby uświadomić sobie, że zamiast wyświetlać listę selected, odpowiednio posortowaną, lepiej wyświetlić listę options ograniczoną do tych jej elementów, które występują w selected. Jest to znacznie prostsze i wydaje się czytelne.

[lgpawel]

To z kolei dobrze, że ja się nie spieszyłem, bo miałem zamiar zaproponować zmianę właśnie w tym duchu 🙂 Teraz jest oczywiście w porządku i działa.

Być może ostatnia rzecz, na którą należałoby zwrócić uwagę, to rzecz z wiadomości/wątku #1325 (comment) (plus ja muszę rozgryźć jeden bardzo techniczny szczegół) i będzie można wreszcie skończyć tę zabawę.

[Telpenarmo]

Zgodnie z naszą rozmową spróbowałem wyekstrahować różnice pomiędzy dodanym w tym PR plikiem scss a domyślnymi stylami MultiSelectFilter.
Wydaje mi się, że udało mi się to osiągnąć, ale niestety, jak wiadomo, CSS nie jest szczególnie przejrzystym językiem, a szukanie dwóch równoważnych styli jest dosyć trudne -- nie mam więc pewności, czy w przepisywaniu czegoś nie pominąłem.
Styli nadpisujących domyślne wyszło mi około 90 linii (licząc puste). Zdecydowałem się umieścić je bezpośrednio w komponencie, ale nie będzie problemem ewentualne przeniesienie ich do oddzielnego pliku.

[lgpawel]

Spodziewałbym się czegoś z grubsza takiego

Suggested change

	<style src="vue-multiselect/dist/vue-multiselect.min.css"></style>
	<style>
	<style lang="scss" scoped>
	@import "~vue-multiselect/dist/vue-multiselect.min.css";

ale proszę samemu sprawdzić/dowiedzieć się, czy ta propozycja (uwzględniając wszystkie jej elementy) ma sens, ja bardziej zgaduję jej formę na podstawie tego, jak wyglądają inne pliki w projekcie. W szczególności nigdzie nie używamy dyrektywy @import w plikach .vue, więc gdyby z jakiegoś kryptycznego powodu nie działała ona tutaj, to będzie należało wywalić to do osobnego pliku .scss (ale poza tą ewentualnością nie widzę w tym momencie takiej potrzeby, optycznie nie wygląda ta sekcja jakoś strasznie).

[Telpenarmo]

Rzeczywiście, wygląda to lepiej i działa.
Problem jest jedynie z atrybutem "scoped": style wypisane w takim bloku nie nakładają się na te zaimportowane. Mogę więc albo usunąć ten atrybut, albo, jeśli miałoby to w czymś pomóc, zrobić dwa bloki: jeden "scoped" z importem, drugi, pozbawiony go, z całą resztą.

[lgpawel]

Ciekawe, z czego to się bierze; zgaduję, że jest ortogonalne do reszty proponowanej przeze mnie zmiany – czy jeśli będą dwa osobne elementy <style> (w "starej" bądź "nowej" formie, to już chyba nie powinno mieć znaczenia), to które z czterech możliwości nadania im tego atrybutu działają, a które nie?

Dwa bloki to oczywiście żadna herezja, jest na to nawet przykład w https://vuejs.org/api/sfc-css-features.html ale dobrze byłoby to robić wtedy, kiedy chcemy, a nie wtedy, kiedy jesteśmy do tego zmuszeni.

[Telpenarmo]

Zgaduję, że problem wynika wprost z semantyki atrybutu scoped: jeśli dobrze rozumiem, to zapewnia on, że style pod nim zadeklarowane obowiązywać będą tylko w komponencie, który ten styl zawiera.
W tym przypadku: komponentem zawierającym styl jest nasz MultiSelectFilter, ale chcemy, aby te style dotyczyły zagnieżdżonego w nim komponentu multiselect.
Dlaczego deklaracja @import działa ze scoped nie rozumiem, ale być może jest jakieś znane silnikowi mapowanie pomiędzy stylami, które importujemy, a komponentem multiselect, dzięki któremu Vue wie, że właśnie w nim powinien te style zastosować.

[lgpawel]

Po porównaniu lokalnego pliku SCSS w ostatnim stanie, w jakim istniał on osobno, i https://github.com/shentao/vue-multiselect/blob/master/src/Multiselect.vue który był podany jako punkt wyjścia do modyfikacji, widzę takie przeoczenia (które być może są celowe, ale wtedy proszę o wyjaśnienie):

• zaginęły dodane u nas ręcznie deklaracje:

  projektzapisy/zapisy/apps/enrollment/timetable/assets/components/filters/MultiSelectFilter.scss

  Line 44 in 405f22a

  display: inline-block;

  projektzapisy/zapisy/apps/enrollment/timetable/assets/components/filters/MultiSelectFilter.scss

  Lines 196 to 199 in 405f22a

  	.multiselect__tag-icon:focus,
  	.multiselect__tag-icon:hover {
  	background: rgb(234, 236, 239);
  	}

• nie wycofujemy obecnych oryginalnie i nie pojawiających się w lokalnym pliku deklaracji https://github.com/shentao/vue-multiselect/blob/c4d69ee5a47592c159f9deb45df9534361ec301c/src/Multiselect.vue#L591 oraz https://github.com/shentao/vue-multiselect/blob/c4d69ee5a47592c159f9deb45df9534361ec301c/src/Multiselect.vue#L690

Dodatkowo trochę nie podoba mi się, że wycofywanie deklaracji, które w oryginalnym stylu się nam nie podobają, odbywa się w bardzo różny sposób...

• zazwyczaj używamy unset:

  projektzapisy/zapisy/apps/enrollment/timetable/assets/components/filters/MultiSelectFilter.vue

  Line 241 in f4477ab

  min-height: unset;

  projektzapisy/zapisy/apps/enrollment/timetable/assets/components/filters/MultiSelectFilter.vue

  Line 268 in f4477ab

  line-height: unset;

  projektzapisy/zapisy/apps/enrollment/timetable/assets/components/filters/MultiSelectFilter.vue

  Lines 291 to 292 in f4477ab

  	margin-bottom: unset;
  	padding-top: unset;

  projektzapisy/zapisy/apps/enrollment/timetable/assets/components/filters/MultiSelectFilter.vue

  Line 303 in f4477ab

  color: unset;

• ale czasami również initial:

  projektzapisy/zapisy/apps/enrollment/timetable/assets/components/filters/MultiSelectFilter.vue

  Lines 258 to 259 in f4477ab

  	padding: initial;
  	margin-bottom: initial;

  (nie umyka mi, że to są deklaracje niejako powtórzone w oryginalnym stylu, więc inne podejście może okazać się zrozumiałe),
• a w paru miejscach konkretnej wartości, być może mniej lub bardziej typowej dla danej własności:

  projektzapisy/zapisy/apps/enrollment/timetable/assets/components/filters/MultiSelectFilter.vue

  Line 245 in f4477ab

  transform: none;

  projektzapisy/zapisy/apps/enrollment/timetable/assets/components/filters/MultiSelectFilter.vue

  Line 296 in f4477ab

  position: fixed;

  projektzapisy/zapisy/apps/enrollment/timetable/assets/components/filters/MultiSelectFilter.vue

  Lines 269 to 270 in f4477ab

  	width: auto;
  	height: auto;

  (przy czym w ostatnim przypadku deklaracje są w jednym bloku z jednym z użyć unset).

Warto byłoby uzasadnić (w komentarzach w kodzie?) te różne podejścia, a jeśli przy okazji uda się je ograniczyć, to tym lepiej.

[Telpenarmo]

Poruszył Pan w swoim komentarzu (o różnicach między moją wersją a tą oryginalną – zawartą w pliku SCSS) wiele wątków, na które trudno byłoby mi odpowiedzieć symetrycznie.
Pozwolę sobie więc odpowiedzieć zmianami w kodzie tam, gdzie zauważone przez Pana różnice są błędne, niepotrzebne lub niespójne (jak np. ustawienie wymiarów w pewnym miejscu na auto – unset działa tam tak samo); komentarzami w kodzie tam, gdzie różnice są wymagane; natomiast w przypadku styli, których ja nie ująłem w swojej wersji, mogę tylko odpowiedzieć tutaj.
Jeśli chodzi o brakujące style klas multiselect__tags* oraz multiselect_spinner*, to nie uwzględniłem ich, ponieważ ani tagów, ani spinnera nie używamy. Nie chciałem zamieszczać martwego kodu – myślę, że jeśli nawet kiedyś te elementy miałyby być użyte, to i tak lepiej, aby ktoś się im przyjrzał.
Nie wycofałem deklaracji min-height: 40px na elementy listy, ponieważ w naszym przypadku ich wysokość jest stała i nieco przekracza te 40px, a wynika z innych styli (line-height, padding itd.).

Wydaje mi się, że to wszystko, ale nie przeczę, że coś mogło mi umknąć, bardzo proszę zapoznać się z poprawkami i komentarzami.

[lgpawel]

Najnowsza zmiana zepsuła jeden detal (commit wcześniej było dobrze), tj. tekst z wielokropkiem wchodzący pod iksa:

Właśnie zobaczyłem też, że cały element jest wyższy niż przycisk do rozwijania dropdowna, ale nie sprawdziłem jeszcze, kiedy to się pojawiło też dopiero w najnowszej wersji. Można to poprawić na dwa sposoby, ale lepiej zmniejszyć wysokość całości do wysokości przycisku – wtedy będzie ona taka sama, jak pola (innego rodzaju) po lewej, na nazwę przedmiotu.

[Telpenarmo]

Rzeczywiście, zbyt frywolnie podszedłem do usuwania kodu. Aż dziwne, że nie tego zauważyłem, pewnie to przez późną godzinę.