Proxy host added to dcm and register a set#55
Merged
Its4Nik merged 6 commits intoui-slider-animation-updatefrom Jan 8, 2026
Merged
Proxy host added to dcm and register a set#55Its4Nik merged 6 commits intoui-slider-animation-updatefrom
Its4Nik merged 6 commits intoui-slider-animation-updatefrom
Conversation
…lient host tracking This commit introduces a new `SlideContent` component for animated UI transitions and enhances the Docker client manager's ability to track host IDs. - **UI Components:** - Added a new `SlideContent` component to `@ui/components/Slides` that utilizes `framer-motion` for animated transitions between different content slides, improving user experience in dynamic interfaces. - Re-exported the `Column` type from `@ui/components/Table` to facilitate easier type imports for consumers. - **Docker Client Manager:** - Implemented `internalListeners` in `DockerClientManagerCore` to proactively manage `WorkerWrapper`'s `hostIds` set. - This new mechanism tracks `host:init`, `host:added`, and `host:removed` events, ensuring the `hostIds` set accurately reflects connected hosts. - The `host:init` event type has been added to `@typings/docker-client`. - **Dockstat Application:** - Refactored `WorkersTable` by removing redundant explicit type annotations from `render` function parameters, leveraging TypeScript's type inference. - Explicit type assertions (`as Type`) were introduced internally within the `render` functions for clarity and type safety where inference was not sufficient.
Contributor
Reviewer's GuideIntroduces an animated SlideContent component and refactors Slides exports, loosens WorkersTable column render typings, and enhances Docker client host tracking by maintaining host ID sets via internal event listeners and updated typings. Sequence diagram for updated host event handling in DockerClientManagerCoresequenceDiagram
actor HostProcess
participant WorkerWrapper
participant DockerClientManagerCore as Manager
participant Logger
HostProcess->>WorkerWrapper: Emit EventMessage(host:added or host:removed)
WorkerWrapper->>Manager: messageListener(msg)
Manager->>Manager: internalListeners(wrapper, msg)
alt msg.type == host:added
Manager->>Logger: debug("Received host:added - adding host to Set")
Manager->>WorkerWrapper: hostIds.add(hostId)
else msg.type == host:removed
Manager->>Logger: debug("Received host:removed - deleting host from Set")
Manager->>WorkerWrapper: hostIds.delete(hostId)
end
Manager->>Manager: triggerHooks(msg)
Manager->>Logger: debug("Triggering hooks for event ...")
Class diagram for Docker client manager host tracking changesclassDiagram
class DockerClientManagerCore {
- workers
+internalListeners(wrapper, msg)
+listenForEvents(params)
+triggerHooks(message)
}
class WorkerWrapper {
+hostIds
+messageListener
}
class Hosts {
}
class EventMessage {
+type
+ctx
}
class DockerClientEvents {
+host_added(ctx)
+host_removed(ctx)
+host_updated(ctx)
+host_health_changed(ctx)
}
DockerClientManagerCore <|-- Hosts
DockerClientManagerCore "1" o--> "*" WorkerWrapper : manages
EventMessage --> DockerClientEvents : type keyof EVENTS
DockerClientManagerCore ..> EventMessage : handles
WorkerWrapper ..> EventMessage : emits
DockerClientManagerCore ..> DockerClientEvents : uses typings
WorkerWrapper --> "*" string : hostIds
Class diagram for updated UI Slides, SlideContent, Table, and WorkersTableclassDiagram
class SlideState {
+activeSlide
+isCollapsed
+animationDirection
+contentHeight
+contentRefs
}
class SlideContent {
+state SlideState
+children
+render()
}
class Slides {
+props SlidesProps
+render()
}
class SlidesHeader {
+header
+render()
}
class MinimalSlidesHeader {
+render()
}
class Table {
+data
+columns Column[]
+render()
}
class Column {
+key
+title
+width
+sortable
+filterable
+align
+render(value)
}
class WorkersTable {
+workers
+render()
}
Slides --> SlideContent : composes
Slides --> SlidesHeader : uses
Slides --> MinimalSlidesHeader : uses
SlideContent ..> SlideState : uses
WorkersTable ..> Table : renders
Table ..> Column : uses
WorkersTable ..> Column : defines_render_functions
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
internalListeners, thehost:initandhost:addedbranches duplicate the same logic; consider extracting a small helper or falling through in the switch to avoid repetition and keep behavior changes synchronized. - The
internalListenersmethod is declaredpublicbut appears to be used only insideDockerClientManagerCore; if it's not meant to be part of the public API, making itprivate/protectedwould communicate intent and reduce surface area. - In
WorkersTable, the removal of typedrenderparameters and the addition of inlineascasts suggests theColumntyping could be improved; consider tightening the genericColumntype so each column’srendercallback receives an appropriately typed value without needing explicit casts.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `internalListeners`, the `host:init` and `host:added` branches duplicate the same logic; consider extracting a small helper or falling through in the switch to avoid repetition and keep behavior changes synchronized.
- The `internalListeners` method is declared `public` but appears to be used only inside `DockerClientManagerCore`; if it's not meant to be part of the public API, making it `private`/`protected` would communicate intent and reduce surface area.
- In `WorkersTable`, the removal of typed `render` parameters and the addition of inline `as` casts suggests the `Column` typing could be improved; consider tightening the generic `Column` type so each column’s `render` callback receives an appropriately typed value without needing explicit casts.
## Individual Comments
### Comment 1
<location> `packages/ui/src/components/Slides/SlideContent.tsx:12-16` </location>
<code_context>
+ state: UseSlideReturn
+ children: Record<string, React.ReactNode>
+}) => {
+ const hasContent =
+ state.activeSlide &&
+ children[state.activeSlide] !== null &&
+ children[state.activeSlide] !== undefined
+ const show = hasContent || !state.isCollapsed
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider making `hasContent` robust to falsy slide keys and using a simpler null check.
The current check `state.activeSlide && children[state.activeSlide] !== null && children[state.activeSlide] !== undefined` will skip valid entries when `activeSlide` is a falsy key (e.g. `""`, "0"). You can avoid relying on key truthiness and simplify the null/undefined check with something like:
```ts
const activeKey = state.activeSlide
const hasContent = activeKey in children && children[activeKey] != null
```
This preserves support for falsy keys and keeps the `null`/`undefined` handling clear.
```suggestion
const activeKey = state.activeSlide
const hasContent =
activeKey in children && children[activeKey] != null
const show = hasContent || !state.isCollapsed
```
</issue_to_address>
### Comment 2
<location> `packages/docker-client/src/manager/core.ts:598-621` </location>
<code_context>
wrapper.messageListener = listener
}
+ public internalListeners(wrapper: WorkerWrapper, msg: EventMessage<keyof EVENTS>) {
+ switch (msg.type) {
+ case "host:init": {
+ this.logger.debug(`Received ${msg.type} - adding host to Set: wrapper.hostIds`)
+ const hostId = (msg.ctx as Parameters<EVENTS["host:added"]>[0]).hostId
+ !wrapper.hostIds.has(hostId) && wrapper.hostIds.add(hostId)
+ break
+ }
+
+ case "host:added": {
+ this.logger.debug(`Received ${msg.type} - adding host to Set: wrapper.hostIds`)
+ const hostId = (msg.ctx as Parameters<EVENTS["host:added"]>[0]).hostId
+ !wrapper.hostIds.has(hostId) && wrapper.hostIds.add(hostId)
+ break
+ }
+
+ case "host:removed": {
+ this.logger.debug(`Received ${msg.type} - removing host from Set: wrapper.hostIds`)
+ const hostId = (msg.ctx as Parameters<EVENTS["host:removed"]>[0]).hostId
</code_context>
<issue_to_address>
**suggestion:** The internal listener switch has duplicated logic and unnecessary `Set` membership checks.
`"host:init"` and `"host:added"` currently share identical behavior, and the `Set.has` checks are unnecessary because `Set.add`/`delete` are idempotent. Combining those cases and relying directly on `add`/`delete` would reduce duplication and avoid future divergence, e.g.:
```ts
public internalListeners(wrapper: WorkerWrapper, msg: EventMessage<keyof EVENTS>) {
switch (msg.type) {
case "host:init":
case "host:added": {
const hostId = (msg.ctx as Parameters<EVENTS[typeof msg.type]>[0]).hostId
this.logger.debug(`Received ${msg.type} - adding host to Set: wrapper.hostIds`)
wrapper.hostIds.add(hostId)
break
}
case "host:removed": {
const hostId = (msg.ctx as Parameters<EVENTS["host:removed"]>[0]).hostId
this.logger.debug("Received host:removed - removing host from Set: wrapper.hostIds")
wrapper.hostIds.delete(hostId)
break
}
}
}
```
```suggestion
public internalListeners(wrapper: WorkerWrapper, msg: EventMessage<keyof EVENTS>) {
switch (msg.type) {
case "host:init":
case "host:added": {
const hostId = (msg.ctx as Parameters<EVENTS[typeof msg.type]>[0]).hostId
this.logger.debug(`Received ${msg.type} - adding host to Set: wrapper.hostIds`)
wrapper.hostIds.add(hostId)
break
}
case "host:removed": {
const hostId = (msg.ctx as Parameters<EVENTS["host:removed"]>[0]).hostId
this.logger.debug("Received host:removed - removing host from Set: wrapper.hostIds")
wrapper.hostIds.delete(hostId)
break
}
}
}
```
</issue_to_address>
### Comment 3
<location> `packages/docker-client/src/manager/core.ts:600-605` </location>
<code_context>
+ public internalListeners(wrapper: WorkerWrapper, msg: EventMessage<keyof EVENTS>) {
+ switch (msg.type) {
+ case "host:init": {
+ this.logger.debug(`Received ${msg.type} - adding host to Set: wrapper.hostIds`)
+ const hostId = (msg.ctx as Parameters<EVENTS["host:added"]>[0]).hostId
+ !wrapper.hostIds.has(hostId) && wrapper.hostIds.add(hostId)
+ break
</code_context>
<issue_to_address>
**suggestion (bug_risk):** `host:init` handler is casting its context as `host:added` which may diverge from the typings.
Here `"host:init"` casts `ctx` as `Parameters<EVENTS["host:added"]>[0]`, even though `"host:init"` has its own typing. This only works because both currently expose `hostId`, but it implicitly ties `"host:init"` to `"host:added"`’s signature and may become incorrect if they diverge. Please use the `"host:init"` event type instead (or `EVENTS[typeof msg.type]` if the cases are merged) to match the actual contract.
```suggestion
case "host:init": {
this.logger.debug(`Received ${msg.type} - adding host to Set: wrapper.hostIds`)
const hostId = (msg.ctx as Parameters<EVENTS["host:init"]>[0]).hostId
!wrapper.hostIds.has(hostId) && wrapper.hostIds.add(hostId)
break
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> Signed-off-by: ItsNik <info@itsnik.de>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> Signed-off-by: ItsNik <info@itsnik.de>
…fecycle logging The 'host:init' event type has been removed from `DockerClientEvents` and its handling logic from `DockerClientManagerCore`. This simplifies the event API and ensures consistency with host lifecycle management. Additionally, debug logging for 'host:added' and 'host:removed' events has been enhanced to include the specific host ID and the exact Set operation (`.add()` or `.delete()`) being performed, improving observability during host lifecycle changes.
Owner
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
DockerClientManagerCore,internalListenersappears to be an implementation detail of the message listener; consider making itprivate(or at least notpublic) and extracting thehostIdfrommsg.ctxvia a small helper to avoid duplicating the cast logic in each case branch. - In
WorkersTable, the loosenedrendertypings now rely on multiplevalue as ...casts; it might be cleaner to adjust theColumn/table generic typings so each column’srendercallback receives a strongly typed value rather than needing runtime-type casts.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `DockerClientManagerCore`, `internalListeners` appears to be an implementation detail of the message listener; consider making it `private` (or at least not `public`) and extracting the `hostId` from `msg.ctx` via a small helper to avoid duplicating the cast logic in each case branch.
- In `WorkersTable`, the loosened `render` typings now rely on multiple `value as ...` casts; it might be cleaner to adjust the `Column`/table generic typings so each column’s `render` callback receives a strongly typed value rather than needing runtime-type casts.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Its4Nik
added a commit
that referenced
this pull request
Jan 8, 2026
* feat(ui/slides): Modularize Slides component and extract state/logic Refactors the `Slides` component into a more maintainable and composable architecture. This commit introduces: - A new `useSlidesState` hook to centralize all slide state management, including active slide, animation direction, and collapse logic. - `ButtonRow` component for rendering slide navigation buttons, supporting both connected and disconnected styles. - `SlidesHeader` and `MinimalSlidesHeader` components to encapsulate header and button row rendering for default and minimal variants, respectively. - `SlideContent` component (implied for content rendering) to handle slide transitions and collapse animations using Framer Motion. - Extraction of Framer Motion variants into `animations.ts` for better organization and improved animation handling (e.g., `position: absolute` for transitions). - Dedicated `types.ts` file for component-specific types. This refactoring significantly improves the separation of concerns, making the `Slides` component easier to understand, maintain, and extend. It also lays the groundwork for more robust and dynamic slide content and collapse animations. * chore(ci): Lint [skip ci] * Proxy host added to dcm and register a set (#55) * feat(multiple): Introduce SlideContent component and enhance Docker client host tracking This commit introduces a new `SlideContent` component for animated UI transitions and enhances the Docker client manager's ability to track host IDs. - **UI Components:** - Added a new `SlideContent` component to `@ui/components/Slides` that utilizes `framer-motion` for animated transitions between different content slides, improving user experience in dynamic interfaces. - Re-exported the `Column` type from `@ui/components/Table` to facilitate easier type imports for consumers. - **Docker Client Manager:** - Implemented `internalListeners` in `DockerClientManagerCore` to proactively manage `WorkerWrapper`'s `hostIds` set. - This new mechanism tracks `host:init`, `host:added`, and `host:removed` events, ensuring the `hostIds` set accurately reflects connected hosts. - The `host:init` event type has been added to `@typings/docker-client`. - **Dockstat Application:** - Refactored `WorkersTable` by removing redundant explicit type annotations from `render` function parameters, leveraging TypeScript's type inference. - Explicit type assertions (`as Type`) were introduced internally within the `render` functions for clarity and type safety where inference was not sufficient. * chore(ci): Lint [skip ci] * Update packages/docker-client/src/manager/core.ts Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> Signed-off-by: ItsNik <info@itsnik.de> * Update packages/ui/src/components/Slides/SlideContent.tsx Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> Signed-off-by: ItsNik <info@itsnik.de> * chore(ci): Lint [skip ci] * refactor(docker-client): Remove 'host:init' event and clarify host lifecycle logging The 'host:init' event type has been removed from `DockerClientEvents` and its handling logic from `DockerClientManagerCore`. This simplifies the event API and ensures consistency with host lifecycle management. Additionally, debug logging for 'host:added' and 'host:removed' events has been enhanced to include the specific host ID and the exact Set operation (`.add()` or `.delete()`) being performed, improving observability during host lifecycle changes. --------- Signed-off-by: ItsNik <info@itsnik.de> Co-authored-by: actions-user <its4nik@users.noreply.github.com> Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> * refactor(ui/slides): Improve slide animation and content handling - Refactor `slideVariants` to utilize Framer Motion's `custom` prop for passing animation direction, simplifying its usage in `SlideContent`. - Introduce `mode="popLayout"` to `AnimatePresence` for more robust and smoother transitions, ensuring better layout management during animations. - Enhance the `hasContent` check in `SlideContent.tsx` to explicitly account for `null` `activeKey` and use `Object.hasOwn` for precise property validation. - Implement null safety when updating `contentRefs.current`, preventing potential runtime errors if `activeSlide` is `null`. - Remove the redundant `previousSlideIndex` ref from `useSlideState`, streamlining state management. --------- Signed-off-by: ItsNik <info@itsnik.de> Co-authored-by: actions-user <its4nik@users.noreply.github.com> Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Handle host add/remove events in the Docker client manager and expose updated UI slide and table components.
New Features:
Bug Fixes:
Enhancements: