Skip to content

Bug/Search: All Categories #31

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

Merged
yuriiShmal merged 16 commits into from
Feb 11, 2026
Merged

Bug/Search: All Categories #31

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
cc45467
Fixed default search to search all categories
yuriiShmal Feb 3, 2026
77b5404
Fixed public/src/client/search.js and
yuriiShmal Feb 7, 2026
99a5c61
Hopefully fixed style
yuriiShmal Feb 7, 2026
edf9896
Linting fix... hopefully.
yuriiShmal Feb 7, 2026
ce4466a
empty commit for deployment
cirex-web Feb 8, 2026
e979328
Fixed display check bug, hopefully fixed doublequerry
yuriiShmal Feb 8, 2026
f3e15f3
Removed unused variable
yuriiShmal Feb 8, 2026
886a3f3
Double query persists, but auto-search works
yuriiShmal Feb 8, 2026
cd750b6
Final fix to search?
yuriiShmal Feb 9, 2026
207b9b2
Reverted Final fix to search? due to undesired
yuriiShmal Feb 10, 2026
2530c2b
Merge branch 'fix-search' of https://github.com/CMU-313/nodebb-spring…
yuriiShmal Feb 10, 2026
0cac967
Search now works. Double search to potentially
yuriiShmal Feb 10, 2026
0e0184e
Fixed redundant categories = categories
yuriiShmal Feb 10, 2026
4ba7c92
Removed flaky url test as advised, reverted search
yuriiShmal Feb 10, 2026
a16f239
Trying alternate fix to flaky test
yuriiShmal Feb 10, 2026
5f9cd81
Fixed to put the plugin tests back in. This is the
yuriiShmal Feb 11, 2026
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
85 changes: 59 additions & 26 deletions public/src/client/search.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ define('forum/search', [
let selectedTags = [];
let selectedCids = [];
let searchFilters = {};
let lastAutoQueryUrl = null;

Search.init = function () {
const searchIn = $('#search-in');
searchIn.on('change', function () {
Expand Down Expand Up @@ -65,6 +67,17 @@ define('forum/search', [
updateSortFilter();

searchFilters = getSearchDataFromDOM();

const currentUrl = window.location.pathname + window.location.search;

// Only auto-query when arriving with a term AND the page doesn't already have results rendered
const hasRenderedResults = !!$('#results .search-results').length;
const term = (searchFilters.term || '').trim();

if (term && !hasRenderedResults && lastAutoQueryUrl !== currentUrl) {
lastAutoQueryUrl = currentUrl;
searchModule.query(searchFilters);
}
Comment on lines +73 to +79
Copy link
Copy Markdown

@cirex-web cirex-web Feb 8, 2026

Choose a reason for hiding this comment

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

this seems to do a double-fetch on any subsequent query (eg. try changing the search text)

Image

was the intention to run search once on load?

(prod version:)

Image

Copy link
Copy Markdown
Author

@yuriiShmal yuriiShmal Feb 8, 2026

Choose a reason for hiding this comment

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

was the intention to run search once on load?

Yep, it was.

};

function updateTagFilter() {
Expand Down Expand Up @@ -122,7 +135,8 @@ define('forum/search', [
if (['posts', 'titlesposts', 'titles', 'bookmarks'].includes(searchData.in)) {
searchData.matchWords = form.find('#match-words-filter').val();
searchData.by = selectedUsers.length ? selectedUsers.map(u => u.username) : undefined;
searchData.categories = selectedCids.length ? selectedCids : undefined;
const cids = Array.isArray(selectedCids) ? selectedCids : [];
searchData.categories = cids.length ? cids : ['all'];
searchData.searchChildren = form.find('#search-children').is(':checked');
searchData.hasTags = selectedTags.length ? selectedTags.map(t => t.value) : undefined;
searchData.replies = form.find('#reply-count').val();
Expand Down Expand Up @@ -252,44 +266,63 @@ define('forum/search', [

function categoryFilterDropdown(_selectedCids) {
ajaxify.data.allCategoriesUrl = '';
selectedCids = _selectedCids || [];

// UI semantics: All = []
let uiCids = Array.isArray(_selectedCids) ? _selectedCids.map(String) : [];

// sanitize any bad inputs
uiCids = uiCids.filter(Boolean);

// if upstream gave ['all'], convert to UI form []
if (uiCids.length === 1 && uiCids[0] === 'all') {
uiCids = [];
}
// if upstream accidentally includes 'all' with others, drop it
if (uiCids.includes('all')) {
uiCids = uiCids.filter(cid => cid !== 'all');
}

ajaxify.data.selectedCids = uiCids;
selectedCids = uiCids;

const dropdownEl = $('[component="category/filter"]');
categoryFilter.init(dropdownEl, {
selectedCids: _selectedCids,
updateButton: false, // prevent categoryFilter module from updating the button
selectedCids: uiCids, // pass [] for All
updateButton: false,
onHidden: async function (data) {
const isActive = data.selectedCids.length > 0 && data.selectedCids[0] !== 'all';
let cids = Array.isArray(data.selectedCids) ? data.selectedCids.map(String) : [];
cids = cids.filter(Boolean).filter(cid => cid !== 'all'); // keep UI form

ajaxify.data.selectedCids = cids;
Copy link
Copy Markdown

@cirex-web cirex-web Feb 8, 2026

Choose a reason for hiding this comment

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

what does ajaxify.data.selectedCids = cids; do exactly?

Copy link
Copy Markdown
Author

@yuriiShmal yuriiShmal Feb 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

It should update the what is displayed on the screen to hopefully be the [] version (with the checkmark) if it is actually ['all'] because its display is coded up to show All Categories on []

Copy link
Copy Markdown

@cirex-web cirex-web Feb 8, 2026

Choose a reason for hiding this comment

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

kk

selectedCids = cids;

const isActive = cids.length > 0;

let labelText = '[[search:categories]]';
ajaxify.data.selectedCids = data.selectedCids;
selectedCids = data.selectedCids;
if (data.selectedCids.length === 1 && data.selectedCids[0] === 'watched') {
if (cids.length === 1 && cids[0] === 'watched') {
ajaxify.data.selectedCategory = { cid: 'watched' };
labelText = `[[search:categories-watched-categories]]`;
} else if (data.selectedCids.length === 1 && data.selectedCids[0] === 'all') {
ajaxify.data.selectedCategory = null;
} else if (data.selectedCids.length > 0) {
const categoryData = await api.get(`/categories/${data.selectedCids[0]}`);
labelText = '[[search:categories-watched-categories]]';
} else if (cids.length > 1) {
labelText = `[[search:categories-x, ${cids.length}]]`;
} else if (cids.length === 1) {
const categoryData = await api.get(`/categories/${cids[0]}`);
ajaxify.data.selectedCategory = categoryData;
labelText = `[[search:categories-x, ${categoryData.name}]]`;
}
if (data.selectedCids.length > 1) {
labelText = `[[search:categories-x, ${data.selectedCids.length}]]`;
} else {
ajaxify.data.selectedCategory = { cid: 'all' };
}

$('[component="category/filter/button"]').toggleClass(
'active-filter', isActive
).find('.filter-label').translateText(labelText);
$('[component="category/filter/button"]')
Copy link
Copy Markdown

@cirex-web cirex-web Feb 8, 2026

Choose a reason for hiding this comment

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

there seems to be a bug where if you close the category dropdown and then reopen it, the checkmark disappears

Image Image

Copy link
Copy Markdown
Author

@yuriiShmal yuriiShmal Feb 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I thought I fixed that. Good to know.
Edit: rolling back the categoryFilterDropdown seems to have fixed the issue; it seems I was trying to fix the issue that wasn't in that location. Which caused problems.

.toggleClass('active-filter', isActive)
.find('.filter-label')
.translateText(labelText);
},
localCategories: [
{
cid: 'watched',
name: '[[category:watched-categories]]',
icon: '',
},
],
localCategories: [{ cid: 'watched', name: '[[category:watched-categories]]', icon: '' }],
});
}



function userFilterDropdown(el, _selectedUsers) {
selectedUsers = _selectedUsers || [];
userFilter.init(el, {
Expand Down
12 changes: 10 additions & 2 deletions public/src/modules/categoryFilter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ define('categoryFilter', ['categorySearch', 'api', 'hooks'], function (categoryS
const icon = categoryEl.find('[component="category/select/icon"]');

if (cid !== 'all') {
if (selectedCids.length === 1 && selectedCids[0] === 'all') {
selectedCids = [];
}

if (selectedCids.includes(cid)) {
selectedCids.splice(selectedCids.indexOf(cid), 1);
} else {
Expand All @@ -86,7 +90,7 @@ define('categoryFilter', ['categorySearch', 'api', 'hooks'], function (categoryS
} else {
el.find('[component="category/select/icon"]').addClass('invisible');
listEl.find('[data-cid="all"] i').removeClass('invisible');
selectedCids = [];
selectedCids = ['all']; // IMPORTANT: backend expects this
}

options.selectedCids = selectedCids;
Expand All @@ -106,7 +110,11 @@ define('categoryFilter', ['categorySearch', 'api', 'hooks'], function (categoryS
bgColor: '#ddd',
});
} else if (selectedCids.length === 1) {
api.get(`/categories/${selectedCids[0]}`, {}).then(renderButton);
if (selectedCids[0] === 'all') {
renderButton(); // default button
} else {
api.get(`/categories/${selectedCids[0]}`, {}).then(renderButton);
}
} else {
renderButton();
}
Expand Down