-
Notifications
You must be signed in to change notification settings - Fork 32
fixed answer feedback icon position #1371
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds dynamic feedback positioning with retrying and MutationObserver-based repositioning, tracks per-message observers/timeouts, wires positioning through retries, updates CSS for stacking/interaction/transition, and cleans up observers when messages or conversations are deleted. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as AnswerFeedback
participant DOM as Message DOM
participant Timer as Retry Timer
participant MO as MutationObserver
Note over UI,DOM #f0f4ff: Positioning flow after message render
UI->>DOM: attachEventListeners(msgKey)
UI->>Timer: retryFeedbackPositioning(msgKey, maxRetries, delay)
Timer-->>UI: attempt positioning
UI->>DOM: read widths (msg-box, rich-text, attachments, text)
alt widths available
UI->>DOM: handleFeedbackPositioning (set left/bottom, show)
UI->>MO: setupMutationObserver(msgKey, container)
MO-->>UI: on DOM changes (debounced)
UI->>DOM: recompute & update positioning
else widths not ready
Timer-->>UI: retry until maxRetries
end
Note over UI,DOM #fff7f0: Cleanup on delete
DOM->>UI: message/conversation deleted
UI->>MO: disconnectObserver(msgKey) / disconnect all
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
webplugin/scss/components/_km-message-area.scss (1)
45-47
: Confirm stacking order; consider transform-based transition for smoother repositioning
- z-index: 10 might still fall under other floating UI (menus/tooltips). Please validate in all chat states (context menu open, attachment previews, etc.).
- Transitioning left causes layout reflow; transform-based transitions are smoother. If keeping left for now, a small hint helps browsers.
Apply this incremental tweak to hint the browser and make the intent explicit:
.km-answer-feedback { position: absolute; bottom: 5px; display: flex !important; justify-content: flex-end; align-items: center; gap: 6px; z-index: 10; pointer-events: auto; transition: left 0.1s ease-out; + will-change: left; }
Optionally, we can switch to translateX for smoother GPU-accelerated transitions and update JS to set transform instead of left. Want me to draft that refactor?
webplugin/js/app/components/answer-feedback-service.js (2)
171-198
: Reduce magic numbers and remove redundant bottom assignment
- The constants 60 (container margin), 40 (icon gap), and 120 (min left) are magic numbers. Make them self-documenting.
- bottom is already set via CSS (Line 40 in SCSS); setting it again on every reposition is redundant.
Proposed cleanup:
- const msgBoxHeight = msgBox.offsetHeight; + const msgBoxHeight = msgBox.offsetHeight; // retained if needed later const richMsgBox = msgContainer.querySelector('.mck-msg-box-rich-text-container'); const attachmentBox = msgContainer.querySelector('.mck-attachment-box'); const msgText = msgContainer.querySelector('.mck-msg-text'); let finalWidth = msgBoxWidth; if (richMsgBox && richMsgBox.offsetWidth > 0) { finalWidth = Math.max(finalWidth, richMsgBox.offsetWidth); } if (attachmentBox && attachmentBox.offsetWidth > 0) { finalWidth = Math.max(finalWidth, attachmentBox.offsetWidth); } if (msgText && msgText.offsetWidth > 0) { finalWidth = Math.max(finalWidth, msgText.offsetWidth); } - const containerWidth = msgContainer.offsetWidth; - const maxAllowedWidth = containerWidth - 60; // Leave some margin - const feedbackWidth = Math.min(Math.max(finalWidth + 40, 120), maxAllowedWidth); - - feedbackElement.style.left = `${feedbackWidth}px`; - if (msgBoxHeight > 0) { - feedbackElement.style.bottom = '5px'; - } + const containerWidth = msgContainer.offsetWidth; + const CONTAINER_MARGIN = 60; // leave some margin with container edges + const ICON_GAP = 40; // spacing between bubble edge and feedback + const MIN_LEFT = 120; // minimum left to avoid overlapping short bubbles + const maxAllowedWidth = containerWidth - CONTAINER_MARGIN; + const feedbackLeft = Math.min(Math.max(finalWidth + ICON_GAP, MIN_LEFT), maxAllowedWidth); + feedbackElement.style.left = `${feedbackLeft}px`; feedbackElement.style.visibility = 'visible';
200-221
: Retry strategy may be too short for slow, async content; make it configurableThree attempts at 100ms can miss late-loading images or rich components. Allow configuration from options and slightly raise defaults.
- retryFeedbackPositioning = (msgKey, maxRetries = 3, delay = 100) => { + retryFeedbackPositioning = ( + msgKey, + maxRetries = this.options?.answerFeedback?.maxRetries ?? 10, + delay = this.options?.answerFeedback?.retryDelay ?? 120 + ) => { let retryCount = 0; const attemptPositioning = () => { const msgContainer = document.querySelector(`[data-msgKey='${msgKey}']`); if (!msgContainer) return; const msgBox = msgContainer.querySelector('.mck-msg-box'); if (!msgBox || msgBox.offsetWidth === 0) { if (retryCount < maxRetries) { retryCount++; - setTimeout(attemptPositioning, delay); + setTimeout(attemptPositioning, delay); } return; } this.handleFeedbackPositioning(msgKey); this.setupMutationObserver(msgKey, msgContainer); }; attemptPositioning(); };If you prefer, we can also provide a requestAnimationFrame-based variant to avoid arbitrary delays.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
webplugin/js/app/components/answer-feedback-service.js
(2 hunks)webplugin/scss/components/_km-message-area.scss
(1 hunks)
🔇 Additional comments (1)
webplugin/js/app/components/answer-feedback-service.js (1)
271-273
: LGTM: deferring positioning until render via retry reduces flakinessSwitching to a retry-based approach is a good call for async content.
To further harden:
- Reposition on window resize (viewport changes) since MutationObserver won’t catch it.
- Verify late-loading images/iframes in rich messages still reposition correctly (ResizeObserver would cover this best).
Want me to add a guarded window resize handler or a ResizeObserver-based implementation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
webplugin/js/app/components/answer-feedback-service.js (1)
90-99
: Prevent infinite loop when walking back to the user’s question; guard nullsIf there is no prior right-floated user message,
question
can becomeundefined
, and the loop will never terminate. Also, dereferencingquestion.msg
will throw whenquestion
is missing.Apply this patch to make the traversal safe and avoid crashes:
- // find the user message - while (!metadata.KM_CONTEXT_QUESTION && question?.floatWhere !== 'mck-msg-right') { - question = question.previousSibling; - } + // find the user message + while (!metadata.KM_CONTEXT_QUESTION && question && question.floatWhere !== 'mck-msg-right') { + question = question.previousSibling; + } @@ - const answerText = answer.msg.message; - const questionText = question.msg.message; + const answerText = answer?.msg?.message || ''; + const questionText = question?.msg?.message || '';
♻️ Duplicate comments (2)
webplugin/js/app/components/answer-feedback-service.js (2)
26-26
: Per-message debounce map added — addresses earlier leak/coordination concernsGood call introducing
this.repositionTimeouts = Object.create(null)
. This resolves the single-shared-timeout issue noted earlier and prevents cross-message interference.
223-264
: Observer callback and per-message debounce look solid
closest(...)
ancestor checks correctly capture deep descendant mutations.- Debouncing per
msgKey
prevents cross-message cancellation.- Disconnecting any prior observer for the same key avoids leaks/duplicates.
Nice work addressing the earlier concerns.
🧹 Nitpick comments (4)
webplugin/js/app/components/answer-feedback-service.js (2)
171-199
: Clamp computed left to avoid negative offsets in narrow containersWhen
msgContainer.offsetWidth < 60
,maxAllowedWidth
becomes negative andleft
can be set to a negative pixel value, pushing the feedback off-canvas.Patch:
- const maxAllowedWidth = containerWidth - 60; // Leave some margin - const feedbackWidth = Math.min(Math.max(finalWidth + 40, 120), maxAllowedWidth); + const maxAllowedWidth = Math.max(containerWidth - 60, 120); // Ensure non-negative and reasonable minimum + const feedbackWidth = Math.min(Math.max(finalWidth + 40, 120), maxAllowedWidth);Please sanity-check on very small iframes/containers (e.g., < 200px wide) to confirm desired behavior.
126-131
: Minor naming nit: helpFulOnClick → helpfulOnClickSpelling consistency aids discoverability. Consider renaming both helpers to
helpfulOnClick
andnotHelpfulOnClick
.Confirm no external callers rely on the current names.
webplugin/js/app/apz-client-1.0.js (2)
81-95
: De-duplicate “disconnect all” logic via a service helperThis loop is repeated elsewhere. To reduce duplication and centralize behavior, add a
disconnectAllObservers()
method onanswerFeedbackService
and call it here.If you prefer not to add a new method, at least extract this loop to a local helper to avoid future drift.
556-571
: Centralize “disconnect all” on conversation delete to reduce duplicationSame suggestion as earlier block: call a
disconnectAllObservers()
on the service instead of re-implementing the loop here.I can draft
disconnectAllObservers()
on the service and update all call sites in this PR if you’d like.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
webplugin/js/app/apz-client-1.0.js
(3 hunks)webplugin/js/app/components/answer-feedback-service.js
(3 hunks)
🔇 Additional comments (5)
webplugin/js/app/components/answer-feedback-service.js (2)
289-291
: LGTM: Defer-and-retry positioning post-renderUsing
retryFeedbackPositioning
before attaching observers is a pragmatic way to handle late layout. Good sequencing.
223-251
: Compatibility check: optional chaining in hot pathsThe observer callback uses
el.closest?.(...)
and other areas use?.
. If this file is shipped without transpilation to ES2019+ targets, older environments (e.g., legacy WebViews) will break.If your build transpiles this file, ignore. Otherwise, add a guard or transpile.
Would you like a patch replacing
?.
with guarded calls for environments without transpilation?webplugin/js/app/apz-client-1.0.js (3)
69-77
: Per-message observer cleanup on delete looks goodThe guard and targeted
disconnectObserver(obj.messageKey)
is correct and idempotent.
535-549
: APPLOZIC_05: immediate per-message cleanup is correctDeriving
messageKey
once and using it for both the event and the cleanup keeps things coherent. Double-callingdisconnectObserver
(here and inonMessageDeleted
) is harmless due to idempotency.
535-538
: ES5 compatibility assured via Babel transpilationThe
webplugin/.babelrc
is configured with@babel/preset-env
targeting IE11, and the project’s build pipeline (viagulp
andgulp-babel
) compiles all JS underwebplugin/
before release. As a result, anyconst
declarations—including those inapz-client-1.0.js
—will be downleveled tovar
for legacy browsers. No change is needed here.
resp.status = data === 'success' ? 'success' : 'error'; | ||
if ( | ||
resp.status === 'success' && | ||
window.Kommunicate && | ||
window.Kommunicate.ui && | ||
window.Kommunicate.ui.answerFeedbackService | ||
) { | ||
window.Kommunicate.ui.answerFeedbackService.disconnectObserver(key); | ||
} | ||
if (typeof callback === 'function') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Fix ReferenceError in deleteConversation success path (key
is undefined)
disconnectObserver(key)
references an undefined identifier. This will throw on successful conversation delete.
Replace with a safe, existing cleanup pattern:
- if (
- resp.status === 'success' &&
- window.Kommunicate &&
- window.Kommunicate.ui &&
- window.Kommunicate.ui.answerFeedbackService
- ) {
- window.Kommunicate.ui.answerFeedbackService.disconnectObserver(key);
- }
+ if (
+ resp.status === 'success' &&
+ window.Kommunicate &&
+ window.Kommunicate.ui &&
+ window.Kommunicate.ui.answerFeedbackService
+ ) {
+ var afs = window.Kommunicate.ui.answerFeedbackService;
+ if (afs.observers) {
+ Object.keys(afs.observers).forEach(function (msgKey) {
+ afs.disconnectObserver(msgKey);
+ });
+ }
+ }
If you can associate observers with a conversation id, prefer filtering instead of clearing all.
🏁 Script executed:
#!/bin/bash
# Verify whether `key` is defined in the deleteConversation success callback
rg -nC5 'disconnectObserver' webplugin/js/app/apz-client-1.0.js
rg -nC5 'deleteConversation' webplugin/js/app/apz-client-1.0.js
Length of output: 3659
Here’s the full deleteConversation
implementation and its success handler for context:
🏁 Script executed:
#!/bin/bash
# Print lines 280–350 to inspect deleteConversation and its callback
sed -n '280,350p' webplugin/js/app/apz-client-1.0.js
Length of output: 3197
Fix undefined key
in deleteConversation success handler
The deleteConversation
AJAX success callback at webplugin/js/app/apz-client-1.0.js:305–314 calls
window.Kommunicate.ui.answerFeedbackService.disconnectObserver(key);
but key
is never declared in that scope, causing a ReferenceError on successful delete.
To resolve:
- Remove the incorrect
disconnectObserver(key)
call. - Instead, use the existing
answerFeedbackService.observers
map to safely disconnect all observers, or—if you have a mapping from conversation IDs to specific observer keys—filter by the relevant ID rather than clearing every observer.
Suggested patch:
- if (
- resp.status === 'success' &&
- window.Kommunicate &&
- window.Kommunicate.ui &&
- window.Kommunicate.ui.answerFeedbackService
- ) {
- window.Kommunicate.ui.answerFeedbackService.disconnectObserver(key);
- }
+ if (
+ resp.status === 'success' &&
+ window.Kommunicate &&
+ window.Kommunicate.ui &&
+ window.Kommunicate.ui.answerFeedbackService
+ ) {
+ var afs = window.Kommunicate.ui.answerFeedbackService;
+ if (afs.observers) {
+ Object.keys(afs.observers).forEach(function (msgKey) {
+ afs.disconnectObserver(msgKey);
+ });
+ }
+ }
If you later introduce a way to link params.id
(conversation or user ID) to observer keys, replace the .forEach
with a filter that only disconnects matching observers.
🤖 Prompt for AI Agents
In webplugin/js/app/apz-client-1.0.js around lines 305 to 314, the success
handler calls
window.Kommunicate.ui.answerFeedbackService.disconnectObserver(key) but `key` is
undefined causing a ReferenceError; remove that call and instead safely
disconnect observers by iterating over
window.Kommunicate.ui.answerFeedbackService.observers (after null checks) and
calling disconnectObserver for each matching observer or for all if no mapping
exists; if you later map params.id to observer keys, replace the iteration with
a filtered iteration that only disconnects observers tied to that conversation
ID.
this.repositionTimeouts = Object.create(null); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Initialize retry timer map in constructor
Add one line under the existing repositionTimeouts
init:
this.msgKeyMap = [];
this.repositionTimeouts = Object.create(null);
+this.retryTimeouts = Object.create(null);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
this.repositionTimeouts = Object.create(null); | |
} | |
constructor() { | |
// existing mappings | |
this.msgKeyMap = []; | |
this.repositionTimeouts = Object.create(null); | |
// initialize retry timer map | |
this.retryTimeouts = Object.create(null); | |
} |
🤖 Prompt for AI Agents
In webplugin/js/app/components/answer-feedback-service.js around lines 26 to 27,
add initialization for the retry timer map directly under the existing
this.repositionTimeouts = Object.create(null); line — e.g. set this.retryTimers
= Object.create(null); — so the property exists before any code references it;
maintain the same placement and spacing as the surrounding constructor code.
retryFeedbackPositioning = (msgKey, maxRetries = 3, delay = 100) => { | ||
let retryCount = 0; | ||
|
||
const attemptPositioning = () => { | ||
const msgContainer = document.querySelector(`[data-msgKey='${msgKey}']`); | ||
if (!msgContainer) return; | ||
|
||
const msgBox = msgContainer.querySelector('.mck-msg-box'); | ||
if (!msgBox || msgBox.offsetWidth === 0) { | ||
if (retryCount < maxRetries) { | ||
retryCount++; | ||
setTimeout(attemptPositioning, delay); | ||
} | ||
return; | ||
} | ||
this.handleFeedbackPositioning(msgKey); | ||
this.setupMutationObserver(msgKey, msgContainer); | ||
}; | ||
|
||
const feedbackWidth = msgBoxWidth + 40; | ||
feedbackElement.style.left = `${feedbackWidth > 120 ? feedbackWidth : 120}px`; | ||
attemptPositioning(); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Track and clear retry timers to avoid dangling timeouts and post-removal callbacks
retryFeedbackPositioning
schedules timeouts that aren’t tracked/cleared. If a message is removed before retries finish, callbacks can run against detached DOM nodes and cause needless work.
- Store retry timer per message (e.g.,
this.retryTimeouts[msgKey]
). - Clear and delete it on success and in
disconnectObserver
.
Patch for this block:
- if (retryCount < maxRetries) {
- retryCount++;
- setTimeout(attemptPositioning, delay);
- }
+ if (retryCount < maxRetries) {
+ retryCount++;
+ // Track per-message retry timer
+ this.retryTimeouts = this.retryTimeouts || Object.create(null);
+ clearTimeout(this.retryTimeouts[msgKey]);
+ this.retryTimeouts[msgKey] = setTimeout(attemptPositioning, delay);
+ }
return;
}
- this.handleFeedbackPositioning(msgKey);
+ this.handleFeedbackPositioning(msgKey);
+ // Clear any outstanding retry timer for this message
+ if (this.retryTimeouts?.[msgKey]) {
+ clearTimeout(this.retryTimeouts[msgKey]);
+ delete this.retryTimeouts[msgKey];
+ }
this.setupMutationObserver(msgKey, msgContainer);
And initialize the map in the constructor (see a separate note below), plus clear it in disconnectObserver
.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
retryFeedbackPositioning = (msgKey, maxRetries = 3, delay = 100) => { | |
let retryCount = 0; | |
const attemptPositioning = () => { | |
const msgContainer = document.querySelector(`[data-msgKey='${msgKey}']`); | |
if (!msgContainer) return; | |
const msgBox = msgContainer.querySelector('.mck-msg-box'); | |
if (!msgBox || msgBox.offsetWidth === 0) { | |
if (retryCount < maxRetries) { | |
retryCount++; | |
setTimeout(attemptPositioning, delay); | |
} | |
return; | |
} | |
this.handleFeedbackPositioning(msgKey); | |
this.setupMutationObserver(msgKey, msgContainer); | |
}; | |
const feedbackWidth = msgBoxWidth + 40; | |
feedbackElement.style.left = `${feedbackWidth > 120 ? feedbackWidth : 120}px`; | |
attemptPositioning(); | |
}; | |
retryFeedbackPositioning = (msgKey, maxRetries = 3, delay = 100) => { | |
let retryCount = 0; | |
const attemptPositioning = () => { | |
const msgContainer = document.querySelector(`[data-msgKey='${msgKey}']`); | |
if (!msgContainer) return; | |
const msgBox = msgContainer.querySelector('.mck-msg-box'); | |
if (!msgBox || msgBox.offsetWidth === 0) { | |
if (retryCount < maxRetries) { | |
retryCount++; | |
// Track per-message retry timer | |
this.retryTimeouts = this.retryTimeouts || Object.create(null); | |
clearTimeout(this.retryTimeouts[msgKey]); | |
this.retryTimeouts[msgKey] = setTimeout(attemptPositioning, delay); | |
} | |
return; | |
} | |
this.handleFeedbackPositioning(msgKey); | |
// Clear any outstanding retry timer for this message | |
if (this.retryTimeouts?.[msgKey]) { | |
clearTimeout(this.retryTimeouts[msgKey]); | |
delete this.retryTimeouts[msgKey]; | |
} | |
this.setupMutationObserver(msgKey, msgContainer); | |
}; | |
attemptPositioning(); | |
}; |
🤖 Prompt for AI Agents
In webplugin/js/app/components/answer-feedback-service.js around lines 201 to
221, retryFeedbackPositioning schedules setTimeouts that aren’t tracked so timed
callbacks can run after a message is removed; store each timeout id on
this.retryTimeouts[msgKey] (ensure this.retryTimeouts is initialized in the
constructor), clear and delete the timeout when positioning succeeds (before
calling handleFeedbackPositioning and setupMutationObserver), and also clear and
delete any pending timeout for that msgKey inside disconnectObserver so no
dangling timeouts fire against removed DOM nodes.
disconnectObserver(msgKey) { | ||
if (this.observers?.[msgKey]) { | ||
this.observers[msgKey].disconnect(); | ||
delete this.observers[msgKey]; | ||
} | ||
if (this.repositionTimeouts?.[msgKey]) { | ||
clearTimeout(this.repositionTimeouts[msgKey]); | ||
delete this.repositionTimeouts[msgKey]; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Also clear retry timers during teardown
To complete the teardown story and avoid late callbacks after message removal:
disconnectObserver(msgKey) {
if (this.observers?.[msgKey]) {
this.observers[msgKey].disconnect();
delete this.observers[msgKey];
}
if (this.repositionTimeouts?.[msgKey]) {
clearTimeout(this.repositionTimeouts[msgKey]);
delete this.repositionTimeouts[msgKey];
}
+ if (this.retryTimeouts?.[msgKey]) {
+ clearTimeout(this.retryTimeouts[msgKey]);
+ delete this.retryTimeouts[msgKey];
+ }
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
disconnectObserver(msgKey) { | |
if (this.observers?.[msgKey]) { | |
this.observers[msgKey].disconnect(); | |
delete this.observers[msgKey]; | |
} | |
if (this.repositionTimeouts?.[msgKey]) { | |
clearTimeout(this.repositionTimeouts[msgKey]); | |
delete this.repositionTimeouts[msgKey]; | |
} | |
} | |
disconnectObserver(msgKey) { | |
if (this.observers?.[msgKey]) { | |
this.observers[msgKey].disconnect(); | |
delete this.observers[msgKey]; | |
} | |
if (this.repositionTimeouts?.[msgKey]) { | |
clearTimeout(this.repositionTimeouts[msgKey]); | |
delete this.repositionTimeouts[msgKey]; | |
} | |
if (this.retryTimeouts?.[msgKey]) { | |
clearTimeout(this.retryTimeouts[msgKey]); | |
delete this.retryTimeouts[msgKey]; | |
} | |
} |
🤖 Prompt for AI Agents
In webplugin/js/app/components/answer-feedback-service.js around lines 266 to
275, the teardown currently clears observers and repositionTimeouts but does not
clear any retry timers, which can cause late callbacks after a message is
removed; modify disconnectObserver to also check this.retryTimers?.[msgKey],
call clearTimeout on it if present, and delete this.retryTimers[msgKey] to fully
cancel pending retries during teardown.
created new |
What do you want to achieve?
PR Checklist
How was the code tested?
What new thing you came across while writing this code?
In case you fixed a bug then please describe the root cause of it?
Screenshot
NOTE: Make sure you're comparing your branch with the correct base branch
Summary by CodeRabbit