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.

Sign up for GitHub

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

Jump to bottom

Refactor/exams page #818

Closed
wants to merge 9 commits into from
Closed

Refactor/exams page #818

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d3e75bb
Refactor exams page
rubuy-74 Oct 7, 2023
d010780
Merge branch 'develop' into refactor/exams-page
rubuy-74 Oct 7, 2023
53da109
Change the filtered exams list state globally
rubuy-74 Oct 13, 2023
7f304dc
Change filteredExamTypes type
rubuy-74 Oct 20, 2023
039eb19
Merge branch 'develop' into refactor/exams-page
bdmendes Dec 5, 2023
89192c6
Extract exam provider updates
bdmendes Dec 8, 2023
9a8e989
Merge branch 'develop' into refactor/exams-page
rubuy-74 Dec 8, 2023
7ccf9a5
Merge branch 'develop' into refactor/exams-page
rubuy-74 Dec 15, 2023
6acbca6
Merge branch 'develop' into refactor/exams-page
rubuy-74 Dec 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions uni/lib/model/entities/exam.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class Exam {
'Recurso': 'ER',
'Especial de Conclusão': 'EC',
'Port.Est.Especiais': 'EE',
'Exames ao abrigo de estatutos especiais': 'EAE'
'Exames ao abrigo de estatutos especiais': 'EAE',
};
static List<String> displayedTypes = types.keys.toList().sublist(0, 4);

Expand All @@ -56,7 +56,7 @@ class Exam {
'end': DateFormat('yyyy-MM-dd HH:mm:ss').format(end),
'rooms': rooms.join(','),
'examType': type,
'faculty': faculty
'faculty': faculty,
};
}

Expand Down
2 changes: 1 addition & 1 deletion uni/lib/view/exams/exams.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class ExamsPageViewState extends GeneralPageViewState<ExamsPageView> {
Column(
children:
createExamsColumn(context, examProvider.getFilteredExams()),
)
),
],
);
},
Expand Down
46 changes: 33 additions & 13 deletions uni/lib/view/exams/widgets/exam_filter_form.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,25 @@ import 'package:uni/model/providers/lazy/exam_provider.dart';

class ExamFilterForm extends StatefulWidget {
const ExamFilterForm(this.filteredExamsTypes, {super.key});
Copy link
Member

Choose a reason for hiding this comment

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

Now make this list an immutable view and create a local version of it in the state class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't the widget.filteredExamTypes already a local version of the immutable view?

Copy link
Member

Choose a reason for hiding this comment

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

The widget is receiving a Map object that is a pointer to a collection of key-value structures probably on the heap. This means that it is mutable.
Consider using UnmodifiableMapView from the collections package. This ensures that you need to create a copy of that map as a member of the state class, which is the best way to do it in my opinion.
Tldr don't modify any widget prop in its lifecycle


final Map<String, bool> filteredExamsTypes;

Map<String, bool> get filteredExamTypes =>
Map<String, bool>.from(filteredExamsTypes)
..removeWhere((key, value) => !Exam.types.containsKey(key));

@override
ExamFilterFormState createState() => ExamFilterFormState();
}

class ExamFilterFormState extends State<ExamFilterForm> {
void _changeFilteredExamList(String key, {bool? value}) {
setState(() {
widget.filteredExamsTypes[key] = value!;
Provider.of<ExamProvider>(context, listen: false)
.setFilteredExams(widget.filteredExamsTypes);
});
Copy link
Member

Choose a reason for hiding this comment

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

You seem to still be modifying the widget prop. Perhaps I could be of help sometime in the next few days so that you get what I was saying?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay.

}

@override
Widget build(BuildContext context) {
return AlertDialog(
Expand All @@ -36,21 +47,34 @@ class ExamFilterFormState extends State<ExamFilterForm> {
.setFilteredExams(widget.filteredExamsTypes);
Navigator.pop(context);
},
)
),
],
content: SizedBox(
height: 230,
width: 200,
child: getExamCheckboxes(widget.filteredExamsTypes, context),
child: FilteredExamList(
widget.filteredExamsTypes,
_changeFilteredExamList,
context,
),
),
);
}
}

class FilteredExamList extends StatelessWidget {
const FilteredExamList(
this.filteredExams,
this.changeFilteredExamList,
this.context, {
super.key,
});
final Map<String, bool> filteredExams;
final void Function(String, {bool? value}) changeFilteredExamList;
final BuildContext context;

Widget getExamCheckboxes(
Map<String, bool> filteredExams,
BuildContext context,
) {
filteredExams.removeWhere((key, value) => !Exam.types.containsKey(key));
@override
Widget build(BuildContext context) {
return ListView(
children: List.generate(filteredExams.length, (i) {
final key = filteredExams.keys.elementAt(i);
Expand All @@ -65,11 +89,7 @@ class ExamFilterFormState extends State<ExamFilterForm> {
),
key: Key('ExamCheck$key'),
value: filteredExams[key],
onChanged: (value) {
setState(() {
filteredExams[key] = value!;
});
},
onChanged: (value) => {changeFilteredExamList(key, value: value)},
);
}),
);
Expand Down
Loading