search

iiuni / projektzapisy

System Zapisów na zajęcia w Instytucie Informatyki Uniwersytetu Wrocławskiego
https://zapisy.ii.uni.wroc.pl
30 stars 10 forks source link

Naprawienie niepoprawnie wyświetlającej się liczby zapisanych osób #1670

Open julgitt opened 3 months ago

julgitt commented 3 months ago

Przed poprawką: image

Po poprawce: image

Pytanie, czy chcemy rozbijać limit na sumę (uwzględniając poszczególne role, jak np. ISIM) czy raczej przedstawić limit jako jedną liczbę (tj. zamiast 14+7, dać 21).

lgpawel commented 3 months ago

Skoro w szablonie była przewidziana jakaś iteracja z taką intencją jak ta, którą teraz realizujemy, i tylko wystarczyło ją naprawić, to oznaczałoby, że ktoś ją kiedyś zepsuł. Prosiłbym o prześledzenie historii zmian (np. przez git bisect) i zidentyfikowanie commitu / PRa, w którym to nastąpiło.

julgitt commented 3 months ago

Link do commita, w którym stworzono logikę gwarantowanych miejsc dla grup (i już wtedy implementacja wyglądała tak, jak przed poprawką): https://github.com/iiuni/projektzapisy/commit/6ad287ab404a14a7d580aa72fd2977b2cdbcdd30

lgpawel commented 2 months ago

Pytanie, czy chcemy rozbijać limit na sumę (uwzględniając poszczególne role, jak np. ISIM) czy raczej przedstawić limit jako jedną liczbę (tj. zamiast 14+7, dać 21).

Wręcz przeciwnie – chcemy rozbić liczbę studentów na sumę podobnie jak w #1648 (modulo szczegóły prezentacji). Co tym bardziej sugeruje, żeby oba problemy rozwiązać jednym PRem, bo są wspólne elementy logiki rozwiązania...

julgitt commented 2 months ago

Tutaj link do commita, który zepsuł wyświetlającą się liczbę osób: https://github.com/iiuni/projektzapisy/commit/bdcabb1dfba5e4e6b2d3071bd5c2f275039f7bd6

lgpawel commented 2 months ago

Co do prezentacji liczb, to ostatecznie dobrze byłoby mieć obie rzeczy naraz, bo różnych użytkowników interesują różne rzeczy (np. studenta – czy są wolne miejsca odpowiednie do jego statusu, a pracownika – jak liczna w ogóle jest grupa). Tak że za cenę pewnej rozwlekłości widziałbym to na tej podstronie jak "N / M, w tym: n0 / m0 niegwarantowanych, n1 / m1 [ISIM], n2 / m2 [Data Science]". Na podstronie z tabelką (trochę intencjonalnie piszę "nie w tym PR" bo nadal uważam, że naturalny będzie jeden) w każdej z dwóch kolumn byłoby "N (n0 + n1 + n2)" gdzie tak jak dotąd składniki sumy poza pierwszym byłyby z tooltipami (problemów z UX, które tu się pojawiają, na razie nie mieszajmy).

julgitt commented 2 months ago

Obecnie pracuję na drugim branchu i zrobię to w jednym pull requeście.

Na ten moment wygląda to w ten sposób: image

image

Teraz przy najechaniu wyświetla się o jaką grupę chodzi, ale mogę oczywiście zmienić, aby było tak, jak było wcześniej (żeby obok był ten ciemniejszy label z nazwą grupy - chyba będzie to czytelniejsze niż obecny "tooltip".

lgpawel commented 2 months ago

Teraz przy najechaniu wyświetla się o jaką grupę chodzi, ale mogę oczywiście zmienić, aby było tak, jak było wcześniej (żeby obok był ten ciemniejszy label z nazwą grupy - chyba będzie to czytelniejsze niż obecny "tooltip".

Tak, labele będą lepsze – tu nie gnieździmy się w małej przestrzeni komórki tabelki, więc nie ma potrzeby ukrywać treści. Proszę jeszcze zwrócić uwagę na takie drobnostki jak np. niepotrzebne spacje pomiędzy nawiasami a treścią w ich środku.