Skip to content

Comments

Add expandable subcategories with lazy loading#6325

Open
EugenBodanov wants to merge 17 commits intosaleor:mainfrom
EugenBodanov:main
Open

Add expandable subcategories with lazy loading#6325
EugenBodanov wants to merge 17 commits intosaleor:mainfrom
EugenBodanov:main

Conversation

@EugenBodanov
Copy link

Motivation Navigating through multiple levels of the category tree just to edit a single item is inefficient and inconvenient for the user. This PR introduces expandable rows, allowing users to access and edit subcategories directly from the parent list without traversing the entire hierarchy.

Key Changes

Categories: Implemented expandable category rows with lazy loading for child items.
Datagrid: Added support for controlled row selection and extended selection hooks to handle partial updates.
Caching: Implemented targeted cache invalidation for updated categories.
DX: Added PORT_DEVSERVER support to override the Vite dev server port.
Chore: Updated translations and bumped eslint-plugin-storybook.

image

@EugenBodanov EugenBodanov requested a review from a team as a code owner February 9, 2026 23:06
@changeset-bot
Copy link

changeset-bot bot commented Feb 9, 2026

🦋 Changeset detected

Latest commit: 21981f2

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@CLAassistant
Copy link

CLAassistant commented Feb 9, 2026

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@codecov
Copy link

codecov bot commented Feb 10, 2026

Codecov Report

❌ Patch coverage is 25.79787% with 279 lines in your changes missing coverage. Please review.
✅ Project coverage is 42.81%. Comparing base (ef11772) to head (7f9b95b).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
src/categories/views/CategoryList/CategoryList.tsx 0.00% 204 Missing and 14 partials ⚠️
...ents/CategoryListDatagrid/CategoryListDatagrid.tsx 65.21% 22 Missing and 2 partials ⚠️
src/components/Datagrid/customCells/ChevronCell.ts 18.51% 22 Missing ⚠️
src/components/Datagrid/Datagrid.tsx 30.76% 8 Missing and 1 partial ⚠️
...s/components/CategoryListPage/CategoryListPage.tsx 58.33% 4 Missing and 1 partial ⚠️
...gories/components/CategoryListDatagrid/datagrid.ts 94.44% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6325      +/-   ##
==========================================
+ Coverage   42.79%   42.81%   +0.02%     
==========================================
  Files        2497     2500       +3     
  Lines       43405    43784     +379     
  Branches    10232    10286      +54     
==========================================
+ Hits        18573    18745     +172     
+ Misses      24795    23708    -1087     
- Partials       37     1331    +1294     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lkostrowski lkostrowski added the test deployment Deploy Pull Request to *.saleor.rocks environment label Feb 10, 2026
@lkostrowski lkostrowski added test deployment Deploy Pull Request to *.saleor.rocks environment and removed test deployment Deploy Pull Request to *.saleor.rocks environment labels Feb 10, 2026
@lkostrowski
Copy link
Member

Thanks for the contribution. I tested locally and it looks good, left minor comments

Extra idea: you may want to store tree state in local storage, so once you go back the tree is restored

@mirekm
Copy link
Member

mirekm commented Feb 10, 2026

Thanks for the contribution @EugenBodanov!

Can you replace the > character with Lucide's chevron (like chevron-right). It should look more or less like this:

import { iconSize, iconStrokeWidthBySize, iconStrokeWidth } from "@dashboard/components/icons";
import { ChevronRight } from "lucide-react";

// Small icons (16px) - use thicker stroke for visibility
<ChevronRight size={iconSize.small} strokeWidth={iconStrokeWidthBySize.small} />

@EugenBodanov
Copy link
Author

EugenBodanov commented Feb 11, 2026

Hi, I replaced the text-based expand indicator with a custom chevron cell in the datagrid.
Note regarding the implementation: Since the DataGrid is canvas-based, we cannot directly render React components (like ChevronRight) inside the cells.

@EugenBodanov
Copy link
Author

@lkostrowski @mirekm Just a heads-up that the PR is ready for review with the requested updates.

@lkostrowski
Copy link
Member

@EugenBodanov please check build

@EugenBodanov
Copy link
Author

@lkostrowski Hi, I've added a type declaration for vite.config.js, so check-types should pass now. I also merged the latest main branch, which should resolve the 'Regression detected: +3 imports' error.

Comment on lines +104 to +115
expect(result).toBe(true);
expect(ctx.save).toHaveBeenCalledTimes(1);
expect(ctx.beginPath).toHaveBeenCalledTimes(1);
expect(ctx.moveTo).toHaveBeenCalledWith(14, 12);
expect(ctx.lineTo).toHaveBeenNthCalledWith(1, 18, 16);
expect(ctx.lineTo).toHaveBeenNthCalledWith(2, 14, 20);
expect(ctx.strokeStyle).toBe("#111111");
expect(ctx.lineWidth).toBe(2);
expect(ctx.lineCap).toBe("round");
expect(ctx.lineJoin).toBe("round");
expect(ctx.stroke).toHaveBeenCalledTimes(1);
expect(ctx.restore).toHaveBeenCalledTimes(1);
Copy link
Member

Choose a reason for hiding this comment

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

this assertion set doesnt make sense, no one will verify internals of how its drawn, you can simplify it to something stable

Comment on lines +6 to +17
const lucideChevronPoints = {
down: [
[6, 9],
[12, 15],
[18, 9],
],
right: [
[9, 6],
[15, 12],
[9, 18],
],
} as const;
Copy link
Member

Choose a reason for hiding this comment

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

is this trying to emulate lucide svg? Idea is fine, but maybe lets add comment what is done here

Comment on lines +4 to +18
describe("allRipples", () => {
it("should include expandable subcategories ripple exactly once", () => {
// Arrange
const matchingRipples = allRipples.filter(
ripple => ripple.ID === rippleExpandedSubcategories.ID,
);

// Act
const count = matchingRipples.length;

// Assert
expect(count).toBe(1);
expect(matchingRipples[0]).toBe(rippleExpandedSubcategories);
});
});
Copy link
Member

Choose a reason for hiding this comment

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

i dont think we need that, you may add generic test that any ripple is at most once

Copy link
Member

Choose a reason for hiding this comment

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

I dont see a point of having this test

@lkostrowski lkostrowski removed the test deployment Deploy Pull Request to *.saleor.rocks environment label Feb 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants