feat: Adding download count badge to sidebar

[oto-ciulis-tt]

Screen.Recording.2024-11-15.074946.mp4

[huchenlei]

This feature needs to be guarded behind isElectron flag.

[oto-ciulis-tt]

This feature needs to be guarded behind isElectron flag.

Since ElectronDownloadStore doesn't populate downloads if it's not running in Electron context I think we don't need it since downloads will never be more than 0 so this badge won't show up. But I can add it for extra safety if you think it's needed.

[huchenlei]

This feature needs to be guarded behind isElectron flag.

Since ElectronDownloadStore doesn't populate downloads if it's not running in Electron context I think we don't need it since downloads will never be more than 0 so this badge won't show up. But I can add it for extra safety if you think it's needed.

useElectronDownloadStore cannot be called in non-electron context as is deconstruct the electronContext to get downloadManager here

ComfyUI_frontend/src/stores/electronDownloadStore.ts

Line 16 in 8af016f

const { DownloadManager } = electronAPI()

I fixed this issue for your previous PR in #1521. This is also why you see all playwright tests failing.

[oto-ciulis-tt]

This feature needs to be guarded behind isElectron flag.

Since ElectronDownloadStore doesn't populate downloads if it's not running in Electron context I think we don't need it since downloads will never be more than 0 so this badge won't show up. But I can add it for extra safety if you think it's needed.

useElectronDownloadStore cannot be called in non-electron context as is deconstruct the electronContext to get downloadManager here

ComfyUI_frontend/src/stores/electronDownloadStore.ts

Line 16 in 8af016f

const { DownloadManager } = electronAPI()

I fixed this issue for your previous PR in #1521. This is also why you see all playwright tests failing.

Wouldn't it be more future-proof to update useElectronDownloadStore to take into account that electronApi can return undefined and update return type of electronApi to reflect that? I think it will be easy to forget to always wrap this with isElectron() test.

[huchenlei]

This feature needs to be guarded behind isElectron flag.

Since ElectronDownloadStore doesn't populate downloads if it's not running in Electron context I think we don't need it since downloads will never be more than 0 so this badge won't show up. But I can add it for extra safety if you think it's needed.

useElectronDownloadStore cannot be called in non-electron context as is deconstruct the electronContext to get downloadManager here

ComfyUI_frontend/src/stores/electronDownloadStore.ts

Line 16 in 8af016f

const { DownloadManager } = electronAPI()

I fixed this issue for your previous PR in #1521. This is also why you see all playwright tests failing.

Wouldn't it be more future-proof to update useElectronDownloadStore to take into account that electronApi can return undefined and update return type of electronApi to reflect that? I think it will be easy to forget to always wrap this with isElectron() test.

I would say we actually want to let it fail if anything not in electron environment tries to call useElectronDownloadStore. At callsite src/hooks/sidebarTabs/modelLibrarySidebarTab.ts we do want future dev know that the badge is only displayed for electron environment, while now this silently depends on the fact that empty downloads just don't show up.