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

Refaktoring komponentów CourseFilter oraz CourseList: Analiza uzycia komponentow CourseFilter oraz CourseList. Dodanie wyjasniajacych komentarzy w tych komponentach. Refaktoring komponentow CourseList w celu zmniejszenia liczby widocznych roznic. Zmiany dotyczą głównie ujednolicenia sposobu deklaracji komponentów Vue i są nieinwazyjne. Porządki w importach dotyczących wyżej wymienionych komponentów. #1709

Open
wants to merge 10 commits into
base: master-dev
Choose a base branch
from

Conversation

rafalstarypan
Copy link
Collaborator

No description provided.

@rafalstarypan
Copy link
Collaborator Author

Ten PR zawiera nastepujace zmiany:

  1. Analiza uzycia komponentow CourseFilter oraz CourseList. Dodanie wyjasniajacych komentarzy w tych komponentach.
  2. Refaktoring komponentow CourseList w celu zmniejszenia liczby widocznych roznic. Zmiany dotyczą głównie ujednolicenia sposobu deklaracji komponentów Vue i są nieinwazyjne.
  3. Porządki w importach dotyczących wyżej wymienionych komponentów.

@rafalstarypan rafalstarypan changed the title Remove unused imports. Add clarifying commments Refaktoring komponentów CourseFilter oraz CourseList: Analiza uzycia komponentow CourseFilter oraz CourseList. Dodanie wyjasniajacych komentarzy w tych komponentach. Refaktoring komponentow CourseList w celu zmniejszenia liczby widocznych roznic. Zmiany dotyczą głównie ujednolicenia sposobu deklaracji komponentów Vue i są nieinwazyjne. Porządki w importach dotyczących wyżej wymienionych komponentów. Jun 14, 2024
Po refaktoryzacji komponentu CourseList.vue występował błąd powodujący brak funkcjonalności checkboxów na liście przedmiotów.
Błąd występował, jak rozumiem, z powodu definiowania property "selection" w elemencie "methods", który służy do definiowania funkcji, przez co komendy "click="selection = []" oraz "v-model:selection" nie mogły zbindować się do property "selection".
Aby nie modyfikować tagu "<template>" przeniosłem definicję property "selection" do elementu "computed". Property w tym elemencie będą automatycznie przeliczane, gdy zmienią się ich zależności (w przypadku "selection" będzie to "this.selectionState"), ale zmiany w this.selectionState są przewidywalne i zgodnie z plikiem "../store/courses.ts" następują wyłącznie po wywołaniu settera "selection", więc nie powinno to powodować niechcianych zachowań.
@jakonradk jakonradk force-pushed the 1679-vue-component-clarification branch from ce8ddf0 to f4784ad Compare November 27, 2024 17:35
Komentarze zostały zamienione na takie, które wskazują odpowiedni webpack, w którym dany komponent jest używany.
…ist w innych miejscach

Komentarze mają na celu wyjaśnienie odpowiednich unikalnych funkcji w każdym z komponentów i zwrócenie uwagi na niejasności.
@@ -13,6 +13,7 @@ import {
MultiselectFilterData,
} from "@/enrollment/timetable/assets/models";
// This component is used in the vote-point-counter webpack
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To chyba właściwsza nomenklatura:

Suggested change
// This component is used in the vote-point-counter webpack
// This component is used in the vote-point-counter asset

(analogicznie w innych miejscach)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Komentarz dałbym zresztą na samym początku pliku czy tam po znaczniku <script> – w niektórych innych komponentach z tego PRa już tam są jakieś inne ogólne komentarze i zwłaszcza w tamtych przypadkach nie ma sensu rozwlekać tego po kilku miejscach w kodzie.

…na stronie głosowania

Komentarze mają na celu wyjaśnienie unikalnej zawartości w komponencie CourseFilter na stronie głosowania w porównaniu do innych komponentów o tej samej nazwie.
@@ -76,6 +77,8 @@ export default Vue.extend({
this.collapsed = false;
}
// this fragment is responsible for
// handling changes in the state
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Analogiczny komentarz przydałby się w timetable/.../CourseList.vue, razem z informacją. co właściwie jest w state (tam: ptaszki postawione przez użytkownika przy przedmiotach, tu: nie pamiętam).

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.

Refaktor i ew. deduplikacja komponentów CourseFilter.vue, CourseList.vue
3 participants