FIX: Make category-drop work with lazy_load_categories #24187
Conversation
nbianca
force-pushed
the
fix_category_drop
branch
from
b2cc5ed
to
0e06559
Compare
nbianca
force-pushed
the
fix_category_drop
branch
2 times, most recently
from
fea5f6b
to
5cf7425
Compare
nbianca
force-pushed
the
fix_category_drop
branch
from
4b3ec65
to
9971771
Compare
app/assets/javascripts/discourse/app/components/bread-crumbs.hbs
Outdated
Show resolved
Hide resolved
app/assets/javascripts/discourse/app/instance-initializers/category-badge-css-generator.js
Outdated
Show resolved
Hide resolved
| const category = this.siteSettings.lazy_load_categories | ||
| ? await Category.asyncFindBySlugPathWithID( | ||
| params.category_slug_path_with_id | ||
| ) |
There was a problem hiding this comment.
In the context of this PR, what problem is this solving? Also, I wonder if we should first check if the category has already been preloaded instead of triggering a network request to fetch the categories unconditionally.
There was a problem hiding this comment.
Yes and no. I mean it is not a direct fix to category-drop, but navigating using category-drop is broken otherwise.
| @@ -5,7 +5,7 @@ class Site | |||
| include ActiveModel::Serialization | |||
|
|
|||
| # Number of categories preloaded when lazy_load_categories is enabled | |||
| LAZY_LOAD_CATEGORIES_LIMIT = 50 | |||
| LAZY_LOAD_CATEGORIES_LIMIT = 10 | |||
There was a problem hiding this comment.
Is there a reason we are reducing the limit here?
There was a problem hiding this comment.
To make other category scaling bugs more obvious, but I can move to a different PR.
| @@ -19,7 +19,7 @@ class BasicCategorySerializer < ApplicationSerializer | |||
| :notification_level, | |||
| :can_edit, | |||
| :topic_template, | |||
| :has_children, | |||
| :has_children?, | |||
There was a problem hiding this comment.
Is ? allowed in the attribute name? Im wondering if there is an existing test for this.
There was a problem hiding this comment.
There was a test, but it was broken. The serializer types were "string/null", but it should've been "bool".
Attribute names containing characters such as a question mark are sanitized and only has_children is returned. The question mark is necessary because we want to serialize category.has_children? (which exists) instead of category.has_children (which does not exist).
nbianca
force-pushed
the
fix_category_drop
branch
2 times, most recently
from
3a341d0
to
541b55d
Compare
nbianca
changed the title
nbianca
force-pushed
the
fix_category_drop
branch
2 times, most recently
from
c40736d
to
46decc7
Compare
nbianca
changed the title
The category drop was rerendered after every category async change because it updated the categories list. This is not necessary and categories can be referenced indirectly by ID instead.
nbianca
force-pushed
the
fix_category_drop
branch
from
46decc7
to
8ab65a7
Compare
The category drop was rerendered after every category async search because it updated the categories list stored in the Site instance. This is not necessary and categories can be referenced indirectly by ID instead to avoid loading objects.
This PR contains other various bug fixes to make it mostly work as expected.