Skip to content

Conversation

@Gozala
Copy link
Contributor

@Gozala Gozala commented Apr 14, 2025

Fixes https://linear.app/common-tools/issue/CT-160/unexpected-refresh-on-charmsspaces-across-tabs by using charmId to track whether to re-render chram view or not.

@Gozala Gozala requested review from Copilot and seefeldb April 14, 2025 23:23
@linear
Copy link

linear bot commented Apr 14, 2025

@Gozala Gozala requested review from bfollington and removed request for Copilot April 14, 2025 23:23
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

jumble/src/components/CharmRunner.tsx:132

  • Verify that casting to 'Cell' aligns with the expected type for the render function, ensuring consistency with the updated type declarations in use.
const cleanup = render(container, charm.asSchema(charmSchema).key(UI) as Cell<VNode>);


// Store a reference to the current charm to detect changes
const prevCharmRef = useRef(charm);
const prevCharmRef = useRef<Charm | null>(null);
Copy link

Copilot AI Apr 14, 2025

Choose a reason for hiding this comment

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

Initializing prevCharmRef to null means the runtime error state will reset on the initial render; if preserving the initial charm state is required, consider initializing with the current charm value.

Suggested change
const prevCharmRef = useRef<Charm | null>(null);
const prevCharmRef = useRef<Charm | null>(charm);

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@bfollington bfollington left a comment

Choose a reason for hiding this comment

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

LGTM

@bfollington bfollington merged commit 79ed441 into main Apr 15, 2025
5 checks passed
@bfollington bfollington deleted the fix/charm-reload branch April 15, 2025 05:38
@seefeldb
Copy link
Contributor

LGTM

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