1.3.3 Sensory Characteristics#1519
Merged
GhiyasKM merged 8 commits intoMay 13, 2026
Merged
Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 631fad71ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR improves accessibility by adding localized labels and bindings, converting non-semantic interactive elements to buttons or button-like roles with ChangesAccessibility Improvements - Button Semantics and Localized Labels
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
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/km-message-markup-1.0.js (1)
14-26:⚠️ Potential issue | 🔴 CriticalFix the attachment label assignment—it crashes when rendering attachments.
Line 17-18 calls
MCK_LABELS['attachment.cancelUpload']as a function, but it's a string. This throws aTypeErroron attachment upload. Additionally,data.retryUploadLabelis never assigned but the template references{{retryUploadLabel}}.Proposed fix
getAttachmentContanier: function (data, fileExpr, isUserMsg, fileUrl) { data.fileExpr = fileExpr; data.fileUrl = fileUrl; - data.cancelUploadLabel = (window.MCK_LABELS && MCK_LABELS['attachment.cancelUpload'])( - window.MCK_LABELS && MCK_LABELS['attachment.retryUpload'] - ); + data.fileMeta = data.fileMeta || {}; + var attachmentLabels = window.MCK_LABELS || {}; + var attachmentFileSuffix = data.fileMeta.name ? ': ' + data.fileMeta.name : ''; + data.cancelUploadLabel = + (attachmentLabels['attachment.cancelUpload'] || 'Cancel upload') + attachmentFileSuffix; + data.retryUploadLabel = + (attachmentLabels['attachment.retryUpload'] || 'Retry upload') + attachmentFileSuffix; data.downloadAttachmentLabel = - (window.MCK_LABELS && MCK_LABELS['attachment.download']) + - (data.fileMeta && data.fileMeta.name ? ': ' + data.fileMeta.name : ''); + (attachmentLabels['attachment.download'] || 'Download attachment') + attachmentFileSuffix; data.previewContainerClass = isUserMsg ? 'km-custom-widget-background-color' : 'km-attach-preview-container'; - data.fileMeta = data.fileMeta || {};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webplugin/js/app/km-message-markup-1.0.js` around lines 14 - 26, In getAttachmentContanier, the code incorrectly invokes MCK_LABELS['attachment.cancelUpload'] as a function and never sets retryUploadLabel, causing a TypeError; change the assignments so cancelUploadLabel reads the string value (not called) and add data.retryUploadLabel = (window.MCK_LABELS && MCK_LABELS['attachment.retryUpload']) || 'Retry' (or another sensible default), and ensure cancelUploadLabel similarly falls back to a default string—update the assignments around data.cancelUploadLabel and data.downloadAttachmentLabel in getAttachmentContanier to use the label values safely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@webplugin/js/app/km-message-markup-1.0.js`:
- Around line 7-8: Replace the SVG-only cancel and retry controls with native
buttons: wrap the cancel SVG (class km-attachment-cancel-icon-{{key}} /
{{cancelIconClass}}) and the upload/retry SVG (class
km-attachment-upload-icon-{{key}} / {{uploadIconClass}}) in <button
type="button"> elements that preserve the existing classes and aria-label/title
values, move/remove role="button" and tabindex from the SVGs onto the button,
and ensure the click handlers (selectors currently targeting those SVG elements)
are updated to target the new button elements so keyboard activation
(Enter/Space) works and the buttons do not submit forms.
- Around line 180-187: The closeButtonLabel is injected unescaped into
closeButtonMarkup and a div with role="button" is used; replace the div-based
control with a native <button> element to restore keyboard activation and remove
the need for custom keyboard handlers, ensure closeButtonLabel is HTML-escaped
before insertion into both aria-label and title, and add aria-hidden="true" to
the SVG markup; update the variable closeButtonMarkup (and any code that renders
it) to use the escaped label for both attributes and a <button> containing the
SVG icon.
In `@webplugin/template/mck-sidebox.html`:
- Around line 167-172: Several interactive divs (e.g.,
id="km-restart-conversation", id="user-overide-voice-output", selectors
.voiceNote, .voiceInput, `#delete-recording`, `#pause-btn`, `#play-btn`, `#send-btn`,
`#mck-stop-recording`, `#mck-voice-repeat-last-msg`) are given role="button" and
tabindex="0" but lack keyboard activation; either replace these divs with
semantic <button type="button"> elements (preferred) or add a delegated keydown
handler that listens for Enter/Space, calls event.preventDefault(), and triggers
the element's click action for those selectors so keyboard users can activate
them.
---
Outside diff comments:
In `@webplugin/js/app/km-message-markup-1.0.js`:
- Around line 14-26: In getAttachmentContanier, the code incorrectly invokes
MCK_LABELS['attachment.cancelUpload'] as a function and never sets
retryUploadLabel, causing a TypeError; change the assignments so
cancelUploadLabel reads the string value (not called) and add
data.retryUploadLabel = (window.MCK_LABELS &&
MCK_LABELS['attachment.retryUpload']) || 'Retry' (or another sensible default),
and ensure cancelUploadLabel similarly falls back to a default string—update the
assignments around data.cancelUploadLabel and data.downloadAttachmentLabel in
getAttachmentContanier to use the label values safely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 73880d20-1486-4bcc-aa8d-978cfd602fb2
📒 Files selected for processing (5)
webplugin/js/app/km-message-markup-1.0.jswebplugin/js/app/kommunicate-ui.jswebplugin/js/app/labels/default-labels-en.jswebplugin/js/app/labels/default-labels.jswebplugin/template/mck-sidebox.html
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@webplugin/js/app/km-message-markup-1.0.js`:
- Around line 17-21: The new attachment label assignments
(data.cancelUploadLabel, data.retryUploadLabel, data.downloadAttachmentLabel)
can become empty or produce "undefined: filename" when window.MCK_LABELS is
missing or keys are not present; update these assignments to use safe fallbacks
(e.g., default strings like "Cancel", "Retry", "Download") when MCK_LABELS or
the specific key is falsy, and build data.downloadAttachmentLabel so it
concatenates the fallback label and the filename only when the label is defined
(avoiding "undefined: <name>"); change the logic in the block that sets
data.cancelUploadLabel, data.retryUploadLabel and data.downloadAttachmentLabel
to prefer MCK_LABELS[...] when present and otherwise substitute the default text
and then append ": " + data.fileMeta.name only if a filename exists.
In `@webplugin/js/app/mck-sidebox-1.0.js`:
- Around line 7016-7024: The keydown handler in bindRoleButtonKeyboardActivation
is causing repeated activations when a key is held; add a guard to ignore
repeated keydown events (event.repeat === true) before calling the activation
callback. Specifically, update the anonymous function bound in
bindRoleButtonKeyboardActivation (and the similar one in
bindDropdownKeyboardActivation) to check event.repeat (and optionally only for
Enter/Space key codes) and return early when true, then call
handleActivationKey(event, this.click.bind(this)) only for non-repeated
keydowns; alternatively move this check into handleActivationKey so both
bindRoleButtonKeyboardActivation and bindDropdownKeyboardActivation are
protected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fddeb606-1cca-4662-889a-cfa57c492ebe
📒 Files selected for processing (5)
webplugin/css/app/style.csswebplugin/js/app/km-message-markup-1.0.jswebplugin/js/app/mck-sidebox-1.0.jswebplugin/scss/components/_km-attachment.scsswebplugin/scss/components/_km-chat-popup.scss
✅ Files skipped from review due to trivial changes (3)
- webplugin/scss/components/_km-chat-popup.scss
- webplugin/scss/components/_km-attachment.scss
- webplugin/css/app/style.css
adityaKommunicate
force-pushed
the
accessibility-assessment-sensory-charaterstics
branch
from
843ac86 to
47cd19a
Compare
| faqBackButton.setAttribute('aria-label', MCK_LABELS['faq.back.to.categories']); | ||
| faqBackButton.setAttribute('title', MCK_LABELS['faq.back.to.categories']); | ||
| } | ||
| ['1', '2', '3', '4', '5'].forEach(function (starValue) { |
Contributor
There was a problem hiding this comment.
do we have rating as 1,2,3,4,5
Author
There was a problem hiding this comment.
yes in mck-sidebox.html->
.... the data-rating="1" , 2 , 3 , 4 , 5
thakaresakshi121
previously approved these changes
Apr 24, 2026
adityaKommunicate
force-pushed
the
accessibility-assessment-sensory-charaterstics
branch
from
269be79 to
2624056
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
webplugin/js/app/components/answer-feedback-service.js (1)
226-230: ⚡ Quick winAdd
aria-hidden="true"to icon SVG containers for better screen reader experience.The SVG icons rendered inside the buttons will be announced by screen readers in addition to the
aria-labeltext, potentially causing redundant or confusing announcements. Since the buttons already have descriptivearia-labelattributes, the icons serve only a visual purpose and should be hidden from assistive technology.♻️ Recommended fix to hide decorative icons
Ensure that the SVG icons in
KommunicateConstants.ANSWER_FEEDBACK_ICONSincludearia-hidden="true"on their root element. If they don't, you can wrap them or add the attribute programmatically:getFeedbackTemplate = () => { return `<button type="button" class="answer-feedback-helpful" aria-label="${ MCK_LABELS.answerFeedback.helpful }" title="${MCK_LABELS.answerFeedback.helpful}"> - ${ + <span aria-hidden="true">${ KommunicateConstants.ANSWER_FEEDBACK_ICONS[ KommunicateConstants.ANSWER_FEEDBACK.HELPFUL ] - } + }</span> </button> <button type="button" class="answer-feedback-not-helpful" aria-label="${ MCK_LABELS.answerFeedback.notHelpful }" title="${MCK_LABELS.answerFeedback.notHelpful}"> - ${ + <span aria-hidden="true">${ KommunicateConstants.ANSWER_FEEDBACK_ICONS[ KommunicateConstants.ANSWER_FEEDBACK.NOT_HELPFUL ] - } + }</span> </button>`; };Alternatively, add
aria-hidden="true"directly to the SVG elements whereANSWER_FEEDBACK_ICONSis defined.Also applies to: 235-239
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webplugin/js/app/components/answer-feedback-service.js` around lines 226 - 230, The SVG icons used from KommunicateConstants.ANSWER_FEEDBACK_ICONS (referenced when rendering KommunicateConstants.ANSWER_FEEDBACK.HELPFUL and the other feedback variants) are decorative and should be hidden from assistive tech; update the SVG root elements in the ANSWER_FEEDBACK_ICONS definitions to include aria-hidden="true" or, if altering the constant is undesirable, ensure the rendering code that injects these icons wraps or sets aria-hidden="true" on the SVG container so screen readers ignore the icons while the button aria-label remains the accessible label.webplugin/scss/components/_km-message-area.scss (1)
116-135: ⚡ Quick winConsider using design system color variables for the focus indicator.
The focus ring and background use a hardcoded pink color (
#ff6b81andrgba(255, 107, 129, 0.08)) that does not appear to come from the established design system variables. Other focus indicators in this file use$primary-colororrgba(76, 102, 205, 0.6), creating a visual inconsistency.If this pink color is intentional for answer feedback controls specifically, consider documenting the decision. Otherwise, using a design system variable (e.g.,
$primary-coloror a custom property like--km-accent) would improve maintainability and ensure consistent theming.♻️ Suggested refactor to use design system color
&::after { content: ''; position: absolute; inset: -2px; - border: 1px solid `#ff6b81`; - box-shadow: 0 0 0 2px `#ff6b81`; + border: 1px solid rgba($primary-color, 0.6); + box-shadow: 0 0 0 2px rgba($primary-color, 0.6); border-radius: inherit; opacity: 0; pointer-events: none; } &:focus, &:focus-visible { - background: rgba(255, 107, 129, 0.08); + background: rgba($primary-color, 0.08); outline: none; &::after { opacity: 1; } }Alternatively, if the pink is intentional, define it as
$answer-feedback-focus-color:#ff6b81;at the top of the file with a comment explaining the design decision.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webplugin/scss/components/_km-message-area.scss` around lines 116 - 135, Replace the hardcoded pink focus colors in the Km message area styles (the &::after pseudo-element and the &:focus / &:focus-visible background) with design-system tokens instead of literal values; update references in the rules shown (the &::after border/box-shadow and the focus background rgba) to use an existing token like $primary-color or a new variable/Custom Property (e.g., $answer-feedback-focus-color or --km-accent) and, if the pink is intentional, add that variable near the top of the stylesheet with a brief comment documenting the decision so theming and consistency are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@webplugin/js/app/components/answer-feedback-service.js`:
- Around line 223-225: The template that builds the answer-feedback buttons
directly uses MCK_LABELS.answerFeedback.helpful and
MCK_LABELS.answerFeedback.notHelpful; change those occurrences to use optional
chaining with sensible fallbacks (e.g. MCK_LABELS?.answerFeedback?.helpful ||
'Helpful' and MCK_LABELS?.answerFeedback?.notHelpful || 'Not helpful') so the
template string won’t throw if labels are missing; apply the same defensive
replacement to the other instance mentioned (the second button/template around
the notHelpful usage).
In `@webplugin/js/app/kommunicate-ui.js`:
- Around line 1304-1312: Guard access to
MCK_LABELS['conversation.header.dropdown'] and the DOM element before setting
innerText: check that MCK_LABELS && MCK_LABELS['conversation.header.dropdown']
exists and that document.getElementById('user-overide-voice-output-text')
returns a non-null element; if the label namespace or specific keys are missing,
fall back to a default string for voiceOutputLabel, and only call
element.innerText after the null check; keep the existing
kommunicateCommons.hide/show and voiceOutputLabel logic but wrap label
resolution and the DOM write in these guards to prevent runtime crashes.
---
Nitpick comments:
In `@webplugin/js/app/components/answer-feedback-service.js`:
- Around line 226-230: The SVG icons used from
KommunicateConstants.ANSWER_FEEDBACK_ICONS (referenced when rendering
KommunicateConstants.ANSWER_FEEDBACK.HELPFUL and the other feedback variants)
are decorative and should be hidden from assistive tech; update the SVG root
elements in the ANSWER_FEEDBACK_ICONS definitions to include aria-hidden="true"
or, if altering the constant is undesirable, ensure the rendering code that
injects these icons wraps or sets aria-hidden="true" on the SVG container so
screen readers ignore the icons while the button aria-label remains the
accessible label.
In `@webplugin/scss/components/_km-message-area.scss`:
- Around line 116-135: Replace the hardcoded pink focus colors in the Km message
area styles (the &::after pseudo-element and the &:focus / &:focus-visible
background) with design-system tokens instead of literal values; update
references in the rules shown (the &::after border/box-shadow and the focus
background rgba) to use an existing token like $primary-color or a new
variable/Custom Property (e.g., $answer-feedback-focus-color or --km-accent)
and, if the pink is intentional, add that variable near the top of the
stylesheet with a brief comment documenting the decision so theming and
consistency are preserved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a302210f-8e2c-47a6-859c-5670ad30b3b1
📒 Files selected for processing (20)
webplugin/css/app/style.csswebplugin/js/app/components/answer-feedback-service.jswebplugin/js/app/km-message-markup-1.0.jswebplugin/js/app/kommunicate-ui.jswebplugin/js/app/labels/default-labels-en.jswebplugin/js/app/labels/default-labels.jswebplugin/js/app/labels/locales/ar.jsonwebplugin/js/app/labels/locales/de.jsonwebplugin/js/app/labels/locales/es.jsonwebplugin/js/app/labels/locales/fr.jsonwebplugin/js/app/labels/locales/hi.jsonwebplugin/js/app/labels/locales/it.jsonwebplugin/js/app/labels/locales/pt.jsonwebplugin/js/app/labels/locales/sv.jsonwebplugin/js/app/labels/locales/ur.jsonwebplugin/js/app/labels/locales/zh.jsonwebplugin/js/app/mck-sidebox-1.0.jswebplugin/js/app/voice/mck-voice.jswebplugin/scss/components/_km-message-area.scsswebplugin/template/mck-sidebox.html
✅ Files skipped from review due to trivial changes (11)
- webplugin/js/app/labels/locales/de.json
- webplugin/js/app/labels/locales/es.json
- webplugin/js/app/labels/locales/it.json
- webplugin/js/app/labels/locales/zh.json
- webplugin/js/app/labels/locales/hi.json
- webplugin/js/app/labels/locales/pt.json
- webplugin/js/app/labels/locales/ar.json
- webplugin/js/app/labels/locales/fr.json
- webplugin/js/app/labels/locales/ur.json
- webplugin/js/app/labels/locales/sv.json
- webplugin/js/app/voice/mck-voice.js
🚧 Files skipped from review as they are similar to previous changes (5)
- webplugin/js/app/km-message-markup-1.0.js
- webplugin/template/mck-sidebox.html
- webplugin/js/app/labels/default-labels-en.js
- webplugin/js/app/mck-sidebox-1.0.js
- webplugin/js/app/labels/default-labels.js
thakaresakshi121
approved these changes
May 13, 2026
3 tasks
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
3 participants
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.
What do you want to achieve?
-Improve the widget’s accessibility for WCAG 1.3.3 by removing sensory/icon-based instructions, adding clearer accessible labels, and making legacy interactive controls keyboard-operable without changing user-facing behavior unnecessarily.
PR Checklist
How was the code tested?
-Verified that the attachment and popup label logic uses safe defaults and valid accessible names.
Verified keyboard activation wiring for focusable legacy role="button" controls in the sidebox logic.
What new thing you came across while writing this code?
-NA
In case you fixed a bug then please describe the root cause of it?
-NA
Screenshot
NOTE: Make sure you're comparing your branch with the correct base branch
Summary by CodeRabbit
New Features
Bug Fixes
Style