fix: show active filter indicators#231
Conversation
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Secrets |  |
Apr 30, 2026 2:57a.m. | Review ↗ |
| Python | |
Apr 30, 2026 2:57a.m. | Review ↗ |
| JavaScript |  |
Apr 30, 2026 2:57a.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces active filter indicators to the user interface, allowing users to see which filters are currently applied and providing a 'Clear all' button. The implementation includes new logic in script.js to track filter states against default values, CSS for styling badges and labels, and HTML updates for both mobile and desktop views. Feedback focuses on improving the robustness of filter summary generation by handling whitespace-only inputs and optimizing performance by passing pre-calculated filter summaries to the rendering functions to avoid redundant processing.
| function getActiveFilterSummaries() { | ||
| return Object.entries(DEFAULT_FILTER_VALUES) | ||
| .map(([id, defaultValue]) => { | ||
| const element = document.getElementById(id); | ||
| if (!element || element.value === defaultValue) return null; | ||
| return `${FILTER_LABELS[id]}: ${getControlDisplayValue(element)}`; | ||
| }) | ||
| .filter(Boolean); | ||
| } |
There was a problem hiding this comment.
The current implementation of getActiveFilterSummaries can produce badges with empty values (e.g., "Search: ") if a user enters only whitespace into a text input. It is better to verify that the display value is not empty before including it in the summary list.
| function getActiveFilterSummaries() { | |
| return Object.entries(DEFAULT_FILTER_VALUES) | |
| .map(([id, defaultValue]) => { | |
| const element = document.getElementById(id); | |
| if (!element || element.value === defaultValue) return null; | |
| return `${FILTER_LABELS[id]}: ${getControlDisplayValue(element)}`; | |
| }) | |
| .filter(Boolean); | |
| } | |
| function getActiveFilterSummaries() { | |
| return Object.entries(DEFAULT_FILTER_VALUES) | |
| .map(([id, defaultValue]) => { | |
| const element = document.getElementById(id); | |
| if (!element || element.value === defaultValue) return null; | |
| const displayValue = getControlDisplayValue(element); | |
| return displayValue ? `${FILTER_LABELS[id]}: ${displayValue}` : null; | |
| }) | |
| .filter(Boolean); | |
| } |
| function renderActiveFilterIndicator(container) { | ||
| if (!container) return; | ||
|
|
||
| const activeFilters = getActiveFilterSummaries(); | ||
| if (!activeFilters.length) { |
There was a problem hiding this comment.
To avoid redundant processing, renderActiveFilterIndicator should accept the pre-calculated activeFilters array as an argument instead of calling getActiveFilterSummaries() internally. This is especially important since this function is called multiple times (for both mobile and desktop containers) on every filter change.
function renderActiveFilterIndicator(container, activeFilters) {
if (!container) return;
if (!activeFilters.length) {| function updateActiveFilterIndicators() { | ||
| renderActiveFilterIndicator(document.getElementById('activeFiltersSummary')); | ||
| renderActiveFilterIndicator( | ||
| document.getElementById('activeFiltersSummaryDesktop'), | ||
| ); | ||
| } |
There was a problem hiding this comment.
Calculate the active filter summaries once and pass them to the rendering functions to improve efficiency and avoid redundant DOM lookups and string processing.
function updateActiveFilterIndicators() {
const activeFilters = getActiveFilterSummaries();
renderActiveFilterIndicator(
document.getElementById('activeFiltersSummary'),
activeFilters,
);
renderActiveFilterIndicator(
document.getElementById('activeFiltersSummaryDesktop'),
activeFilters,
);
}
|
Updated per the review feedback: whitespace-only text inputs no longer render empty active-filter badges, and active filter summaries are now calculated once and passed to both indicator renderers. Re-ran |
There was a problem hiding this comment.
Pull request overview
Adds an “Active filters / sort” summary to the developer list UI so users can quickly understand why the results are filtered/sorted, with a one-click “Clear all” action.
Changes:
- Adds active filter/sort summary containers to both sidebar and desktop results areas.
- Introduces JS logic to compute active filter badges and render them (plus a “Clear all” button), reusing a shared default-values map for reset + summary.
- Adds CSS styles for the summary badges and clear action in sidebar and desktop contexts.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| layouts/layout.html | Adds DOM placeholders for active filter/sort summaries in sidebar + desktop results areas. |
| docs/styles.css | Styles the active filter summary container, badges, and “Clear all” button (including desktop overrides). |
| docs/script.js | Computes active filter/sort summaries from current control values and renders badges; reuses shared defaults for reset. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function updateVisibilityAndSort() { | ||
| const sortBy = document.getElementById('sortBy').value; | ||
| const sortedUsers = getSortedUsers(sortBy); | ||
|
|
||
| renderCards(sortedUsers); | ||
| updateCounts(sortedUsers); | ||
| updateResultsMessage(sortedUsers); | ||
| updateActiveFilterIndicators(); | ||
| } |
There was a problem hiding this comment.
updateActiveFilterIndicators() now shows the currently selected sort label as an active badge, but the sortBy <select> includes followers-growth-desc / followers-growth-asc options (in layouts/layout.html) that are not implemented in getSortedUsers()’s sorters map. Selecting those sorts will display an active sort badge even though no corresponding sort is applied. Either implement these two sorters or remove/disable the unsupported options to keep the indicator accurate.
| function getActiveFilterSummaries() { | ||
| return Object.entries(DEFAULT_FILTER_VALUES) | ||
| .map(([id, defaultValue]) => { | ||
| const element = document.getElementById(id); | ||
| if (!element || element.value === defaultValue) return null; | ||
| const displayValue = getControlDisplayValue(element); | ||
| return displayValue ? `${FILTER_LABELS[id]}: ${displayValue}` : null; | ||
| }) | ||
| .filter(Boolean); |
There was a problem hiding this comment.
getActiveFilterSummaries() trims the displayed value for text inputs, but it decides “active vs default” using the raw element.value. For searchInput, the filter logic currently uses .value.toLowerCase() without .trim() (see getActiveFilters()), so whitespace-only input will (a) filter out results while (b) producing no active badge because displayValue becomes empty and is dropped. Consider trimming searchInput in the filter logic and using the same trimmed value when determining whether a text input deviates from its default, so the indicator and filtering behavior stay consistent.
|
Hi! Just checking in — please let me know if any changes are needed. Happy to update the PR. Thanks! 🙂 |
|
Hey @santoslgl01-web you have earned $1 for this PR We pay with GitHub Sponsors so you will need a profile |
Summary
Fixes #145
Adds a visible active filter/sort indicator so users can immediately see why the current developer list is narrowed or ordered differently.
Changes
Testing
python3 -m py_compile render.py fetch.py generate_featured.pypython3 render.py(HTML generated; existing sitemap safe-path warning still appears)node --check docs/script.jspython3 -m http.server: enteringsindreshowedActive: Search: sindreand filtered the list to 1 result with no console errors