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

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

Open
wants to merge 1 commit into
base: master-dev
Choose a base branch
from

Conversation

julgitt
Copy link
Collaborator

@julgitt julgitt commented Mar 22, 2024

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
Copy link
Contributor

lgpawel commented Apr 3, 2024

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
Copy link
Collaborator Author

julgitt commented Apr 12, 2024

Link do commita, w którym stworzono logikę gwarantowanych miejsc dla grup (i już wtedy implementacja wyglądała tak, jak przed poprawką): 6ad287a

@lgpawel
Copy link
Contributor

lgpawel commented Apr 26, 2024

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
Copy link
Collaborator Author

julgitt commented Apr 26, 2024

Tutaj link do commita, który zepsuł wyświetlającą się liczbę osób: bdcabb1

@lgpawel
Copy link
Contributor

lgpawel commented May 17, 2024

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
Copy link
Collaborator Author

julgitt commented May 17, 2024

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
Copy link
Contributor

lgpawel commented May 17, 2024

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.

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.

Niepoprawna maksymalna liczba osób zapisanych na przedmiot
2 participants