Skip to content

Commit bd49ec0

Browse files
committed
refactor(feed): refactor SourceFilterPage to use dedicated filter UI
Refactors the `SourceFilterPage` to improve user experience and fix a filtering bug. The UI for selecting headquarters and source types has been moved to a new, dedicated `SourceListFilterPage`. - A new "Filter" icon in the `AppBar` now navigates to the `SourceListFilterPage`. - The main page now only displays the list of sources, which is filtered based on the criteria returned from the new filter page. - The complex and buggy local UI for `ChoiceChip` filters has been removed, simplifying the page's state management and resolving the issue where selecting a source type would incorrectly select all sources.
1 parent 3f7ef81 commit bd49ec0

File tree

1 file changed

+53
-191
lines changed

1 file changed

+53
-191
lines changed

lib/headlines-feed/view/source_filter_page.dart

Lines changed: 53 additions & 191 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import 'package:flutter_bloc/flutter_bloc.dart';
66
import 'package:flutter_news_app_mobile_client_full_source_code/headlines-feed/bloc/headlines_filter_bloc.dart';
77
import 'package:flutter_news_app_mobile_client_full_source_code/l10n/app_localizations.dart';
88
import 'package:flutter_news_app_mobile_client_full_source_code/l10n/l10n.dart';
9+
import 'package:flutter_news_app_mobile_client_full_source_code/router/routes.dart';
10+
import 'package:go_router/go_router.dart';
911
import 'package:ui_kit/ui_kit.dart';
1012

1113
/// {@template source_filter_page}
@@ -43,10 +45,11 @@ class _SourceFilterView extends StatefulWidget {
4345
}
4446

4547
class _SourceFilterViewState extends State<_SourceFilterView> {
46-
// Local state for the headquarter country filter capsules.
47-
// This is intentionally decoupled from the main filter bloc's country
48-
// selection, which is for "event country".
49-
final Set<Country> _selectedHeadquarterCountries = {};
48+
// Local state to hold the filter criteria for the source list.
49+
// These are managed by the dedicated SourceListFilterPage and are used
50+
// only for filtering the UI in this page, not for the final headline query.
51+
Set<Country> _filteredHeadquarterCountries = {};
52+
Set<SourceType> _filteredSourceTypes = {};
5053

5154
@override
5255
Widget build(BuildContext context) {
@@ -61,6 +64,39 @@ class _SourceFilterViewState extends State<_SourceFilterView> {
6164
style: textTheme.titleLarge,
6265
),
6366
actions: [
67+
// Filter button to open the dedicated filter page.
68+
IconButton(
69+
icon: const Icon(Icons.filter_alt_outlined),
70+
tooltip: l10n.sourceListFilterPageFilterButtonTooltip,
71+
onPressed: () async {
72+
final filterState = context.read<HeadlinesFilterBloc>().state;
73+
final result = await context.pushNamed<Map<String, dynamic>>(
74+
Routes.sourceListFilterName,
75+
extra: {
76+
'allCountries': filterState.allCountries,
77+
'allSourceTypes':
78+
filterState.allSources
79+
.map((s) => s.sourceType)
80+
.toSet()
81+
.toList()
82+
..sort((a, b) => a.name.compareTo(b.name)),
83+
'initialSelectedHeadquarterCountries':
84+
_filteredHeadquarterCountries,
85+
'initialSelectedSourceTypes': _filteredSourceTypes,
86+
},
87+
);
88+
89+
// When the filter page returns with new criteria, update the
90+
// local state to re-render the list.
91+
if (result != null && mounted) {
92+
setState(() {
93+
_filteredHeadquarterCountries =
94+
result['countries'] as Set<Country>;
95+
_filteredSourceTypes = result['types'] as Set<SourceType>;
96+
});
97+
}
98+
},
99+
),
64100
// Apply Filters Button (now just pops, as state is managed centrally)
65101
IconButton(
66102
icon: const Icon(Icons.check),
@@ -107,17 +143,17 @@ class _SourceFilterViewState extends State<_SourceFilterView> {
107143

108144
// Filter sources based on selected countries and types from HeadlinesFilterBloc
109145
final displayableSources = filterState.allSources.where((source) {
110-
// Use the local state for headquarter country filtering.
146+
// Filter by headquarters country.
111147
final matchesCountry =
112-
_selectedHeadquarterCountries.isEmpty ||
113-
_selectedHeadquarterCountries.any(
148+
_filteredHeadquarterCountries.isEmpty ||
149+
_filteredHeadquarterCountries.any(
114150
(c) => c.isoCode == source.headquarters.isoCode,
115151
);
116152

117-
// Assuming all source types are available and selected by default if none are explicitly selected
153+
// Filter by source type.
118154
final matchesType =
119-
filterState.selectedSources.isEmpty ||
120-
filterState.selectedSources.contains(source);
155+
_filteredSourceTypes.isEmpty ||
156+
_filteredSourceTypes.contains(source.sourceType);
121157
return matchesCountry && matchesType;
122158
}).toList();
123159

@@ -135,193 +171,19 @@ class _SourceFilterViewState extends State<_SourceFilterView> {
135171
);
136172
}
137173

138-
return Column(
139-
crossAxisAlignment: CrossAxisAlignment.start,
140-
children: [
141-
_buildCountryCapsules(
142-
context,
143-
filterState.allCountries,
144-
l10n,
145-
textTheme,
146-
),
147-
const SizedBox(height: AppSpacing.md),
148-
_buildSourceTypeCapsules(context, filterState, l10n, textTheme),
149-
const SizedBox(height: AppSpacing.md),
150-
Padding(
151-
padding: const EdgeInsets.symmetric(
152-
horizontal: AppSpacing.paddingMedium,
153-
),
154-
child: Text(
155-
l10n.headlinesFeedFilterSourceLabel,
156-
style: textTheme.titleMedium?.copyWith(
157-
fontWeight: FontWeight.bold,
158-
),
159-
),
160-
),
161-
const SizedBox(height: AppSpacing.sm),
162-
Expanded(
163-
child: _buildSourcesList(
164-
context,
165-
filterState,
166-
l10n,
167-
textTheme,
168-
displayableSources,
169-
),
170-
),
171-
],
174+
// The main content is now just the list of sources.
175+
return _buildSourcesList(
176+
context,
177+
filterState,
178+
l10n,
179+
textTheme,
180+
displayableSources,
172181
);
173182
},
174183
),
175184
);
176185
}
177186

178-
Widget _buildCountryCapsules(
179-
BuildContext context,
180-
List<Country> allCountries,
181-
AppLocalizations l10n,
182-
TextTheme textTheme,
183-
) {
184-
return Padding(
185-
padding: const EdgeInsets.symmetric(
186-
horizontal: AppSpacing.paddingMedium,
187-
).copyWith(top: AppSpacing.md),
188-
child: Column(
189-
crossAxisAlignment: CrossAxisAlignment.start,
190-
children: [
191-
Text(
192-
l10n.headlinesFeedFilterSourceCountryLabel,
193-
style: textTheme.titleMedium?.copyWith(fontWeight: FontWeight.bold),
194-
),
195-
const SizedBox(height: AppSpacing.sm),
196-
SizedBox(
197-
height: AppSpacing.xl + AppSpacing.md,
198-
child: ListView.separated(
199-
scrollDirection: Axis.horizontal,
200-
itemCount: allCountries.length + 1,
201-
separatorBuilder: (context, index) =>
202-
const SizedBox(width: AppSpacing.sm),
203-
itemBuilder: (context, index) {
204-
if (index == 0) {
205-
return ChoiceChip(
206-
label: Text(l10n.headlinesFeedFilterAllLabel),
207-
labelStyle: textTheme.labelLarge,
208-
selected: _selectedHeadquarterCountries.isEmpty,
209-
onSelected: (_) {
210-
// Clear all country selections
211-
if (_selectedHeadquarterCountries.isNotEmpty) {
212-
setState(_selectedHeadquarterCountries.clear);
213-
}
214-
},
215-
);
216-
}
217-
final country = allCountries[index - 1];
218-
return ChoiceChip(
219-
avatar: country.flagUrl.isNotEmpty
220-
? CircleAvatar(
221-
backgroundImage: NetworkImage(country.flagUrl),
222-
radius: AppSpacing.sm + AppSpacing.xs,
223-
)
224-
: null,
225-
label: Text(country.name),
226-
labelStyle: textTheme.labelLarge,
227-
selected: _selectedHeadquarterCountries.contains(country),
228-
onSelected: (isSelected) {
229-
setState(() {
230-
if (isSelected) {
231-
_selectedHeadquarterCountries.add(country);
232-
} else {
233-
_selectedHeadquarterCountries.remove(country);
234-
}
235-
});
236-
},
237-
);
238-
},
239-
),
240-
),
241-
],
242-
),
243-
);
244-
}
245-
246-
Widget _buildSourceTypeCapsules(
247-
BuildContext context,
248-
HeadlinesFilterState filterState,
249-
AppLocalizations l10n,
250-
TextTheme textTheme,
251-
) {
252-
// For source types, we need to get all unique source types from all available sources
253-
final allSourceTypes =
254-
filterState.allSources.map((s) => s.sourceType).toSet().toList()
255-
..sort((a, b) => a.name.compareTo(b.name));
256-
257-
// Determine which source types are currently selected based on the selected sources
258-
final selectedSourceTypes = filterState.selectedSources
259-
.map((s) => s.sourceType)
260-
.toSet();
261-
262-
return Padding(
263-
padding: const EdgeInsets.symmetric(horizontal: AppSpacing.paddingMedium),
264-
child: Column(
265-
crossAxisAlignment: CrossAxisAlignment.start,
266-
children: [
267-
Text(
268-
l10n.headlinesFeedFilterSourceTypeLabel,
269-
style: textTheme.titleMedium?.copyWith(fontWeight: FontWeight.bold),
270-
),
271-
const SizedBox(height: AppSpacing.sm),
272-
SizedBox(
273-
height: AppSpacing.xl + AppSpacing.md,
274-
child: ListView.separated(
275-
scrollDirection: Axis.horizontal,
276-
itemCount: allSourceTypes.length + 1,
277-
separatorBuilder: (context, index) =>
278-
const SizedBox(width: AppSpacing.sm),
279-
itemBuilder: (context, index) {
280-
if (index == 0) {
281-
return ChoiceChip(
282-
label: Text(l10n.headlinesFeedFilterAllLabel),
283-
labelStyle: textTheme.labelLarge,
284-
selected: selectedSourceTypes.isEmpty,
285-
onSelected: (_) {
286-
// Clear all source selections
287-
for (final source in filterState.allSources) {
288-
context.read<HeadlinesFilterBloc>().add(
289-
FilterSourceToggled(
290-
source: source,
291-
isSelected: false,
292-
),
293-
);
294-
}
295-
},
296-
);
297-
}
298-
final sourceType = allSourceTypes[index - 1];
299-
return ChoiceChip(
300-
label: Text(sourceType.name),
301-
labelStyle: textTheme.labelLarge,
302-
selected: selectedSourceTypes.contains(sourceType),
303-
onSelected: (isSelected) {
304-
// Toggle all sources of this type
305-
for (final source in filterState.allSources.where(
306-
(s) => s.sourceType == sourceType,
307-
)) {
308-
context.read<HeadlinesFilterBloc>().add(
309-
FilterSourceToggled(
310-
source: source,
311-
isSelected: isSelected,
312-
),
313-
);
314-
}
315-
},
316-
);
317-
},
318-
),
319-
),
320-
],
321-
),
322-
);
323-
}
324-
325187
Widget _buildSourcesList(
326188
BuildContext context,
327189
HeadlinesFilterState filterState,

0 commit comments

Comments
 (0)