fix: Slider filled track alignment in RTL mode#38875
Conversation
Ensures consistent brand naming across the project documentation. Fixed a typo where 'Youtube' was used instead of 'YouTube'.
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
|
|
WalkthroughAdds a client-side startup hook that injects a single style tag to flip the RCX slider track for RTL layouts, and a minor README text change. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Meteor Client Startup
participant Module as sliderRTLFix module
participant Util as injectSliderRTLFix()
participant DOM as Browser document.head
Client->>Module: import/execute on startup
Module->>Util: call injectSliderRTLFix()
Util->>DOM: query for element id "rcx-slider-rtl-fix"
alt element not found
Util->>DOM: create <style id="rcx-slider-rtl-fix"> with RTL CSS
Util->>DOM: append style to document.head
else element exists
Util-->>DOM: no-op (avoid duplicate injection)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 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. 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/meteor/client/lib/utils/injectSliderRTLFix.ts (1)
8-12: UsetextContentinstead ofinnerHTMLfor style element content.While this content is fully hardcoded and poses no actual XSS risk,
textContentis the correct and idiomatic API for setting text content on a<style>element.innerHTMLtriggers HTML parsing, whereastextContentinserts a direct text node — which is exactly what CSS injection needs.♻️ Proposed refactor
- styleElement.innerHTML = ` - [dir='rtl'] .rcx-slider__track::before { - transform: scaleX(-1); - } - `; + styleElement.textContent = ` + [dir='rtl'] .rcx-slider__track::before { + transform: scaleX(-1); + } + `;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/lib/utils/injectSliderRTLFix.ts` around lines 8 - 12, Replace the use of styleElement.innerHTML with styleElement.textContent in the injectSliderRTLFix implementation: locate the assignment to styleElement.innerHTML that injects the CSS rule for "[dir='rtl'] .rcx-slider__track::before" and set it via styleElement.textContent instead so the CSS is inserted as text rather than parsed HTML (keep the same CSS string and leave the rest of the style element creation logic unchanged).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.mdapps/meteor/client/lib/utils/injectSliderRTLFix.tsapps/meteor/client/startup/index.tsapps/meteor/client/startup/sliderRTLFix.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/lib/utils/injectSliderRTLFix.tsapps/meteor/client/startup/sliderRTLFix.tsapps/meteor/client/startup/index.ts
🧠 Learnings (4)
📚 Learning: 2026-02-10T16:32:42.586Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:42.586Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON and Date fields arrive as Date objects; do not manually construct new Date() in stream handlers (for example, in sdk.stream()). Only REST API responses return plain JSON where dates are strings, so implement explicit conversion there if needed. Apply this guidance to all TypeScript files under apps/meteor/client to ensure consistent date handling in DDP streams and REST responses.
Applied to files:
apps/meteor/client/lib/utils/injectSliderRTLFix.tsapps/meteor/client/startup/sliderRTLFix.tsapps/meteor/client/startup/index.ts
📚 Learning: 2026-01-08T15:03:59.621Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38068
File: apps/meteor/tests/data/apps/app-packages/README.md:14-16
Timestamp: 2026-01-08T15:03:59.621Z
Learning: For the RocketChat/Rocket.Chat repository, do not analyze or report formatting issues (such as hard tabs vs spaces, line breaks, etc.). The project relies on automated linting tools to enforce formatting standards.
Applied to files:
README.md
📚 Learning: 2025-11-10T19:06:20.146Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
Applied to files:
README.md
📚 Learning: 2025-12-02T22:23:49.593Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37654
File: apps/meteor/client/hooks/useAppSlashCommands.ts:32-38
Timestamp: 2025-12-02T22:23:49.593Z
Learning: In apps/meteor/client/hooks/useAppSlashCommands.ts, the `data?.forEach((command) => slashCommands.add(command))` call during render is intentional. The query is configured with `structuralSharing: false` to prevent React Query from keeping stable data references, and `slashCommands.add` is idempotent, so executing on every render is acceptable and ensures the command registry stays current.
Applied to files:
apps/meteor/client/startup/index.ts
🧬 Code graph analysis (1)
apps/meteor/client/startup/sliderRTLFix.ts (1)
apps/meteor/client/lib/utils/injectSliderRTLFix.ts (1)
injectSliderRTLFix(1-14)
🪛 ast-grep (0.40.5)
apps/meteor/client/lib/utils/injectSliderRTLFix.ts
[warning] 7-11: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: styleElement.innerHTML = [dir='rtl'] .rcx-slider__track::before { transform: scaleX(-1); }
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 7-11: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: styleElement.innerHTML = [dir='rtl'] .rcx-slider__track::before { transform: scaleX(-1); }
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cubic · AI code reviewer
🔇 Additional comments (3)
README.md (1)
118-118: LGTM!Correct capitalization of "YouTube".
apps/meteor/client/startup/sliderRTLFix.ts (1)
1-7: LGTM!Clean Meteor startup hook. Calling
injectSliderRTLFix()unconditionally is correct — the[dir='rtl']selector in the injected CSS handles RTL-scoping at the CSS level, so injecting in LTR contexts is harmless.apps/meteor/client/startup/index.ts (1)
11-11: LGTM!Import correctly wires the new startup module into the client startup sequence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/client/lib/utils/injectSliderRTLFix.ts`:
- Around line 9-10: The injected selector targets a non-existent pseudo-element
and thus does nothing; update the RTL fix in injectSliderRTLFix.ts to target the
real DOM element (.rcx-slider-track) instead of .rcx-slider__track::before, and
replace the naive transform flip with a combination of (a) overriding
positioning using logical properties (use inset-inline-start/inset-inline-end or
set right/inset-inline-end to mirror the inline positioning instead of relying
on left) to counteract inline left/width inline styles, and (b) if the slider
value->position math is computed in JS, invert that calculation for RTL at the
source (where the track width/left are computed) so the fill segment reflects
the correct value direction; ensure any overrides have sufficient specificity
(or !important) to supersede inline styles.
---
Nitpick comments:
In `@apps/meteor/client/lib/utils/injectSliderRTLFix.ts`:
- Around line 8-12: Replace the use of styleElement.innerHTML with
styleElement.textContent in the injectSliderRTLFix implementation: locate the
assignment to styleElement.innerHTML that injects the CSS rule for "[dir='rtl']
.rcx-slider__track::before" and set it via styleElement.textContent instead so
the CSS is inserted as text rather than parsed HTML (keep the same CSS string
and leave the rest of the style element creation logic unchanged).
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Adds a global CSS patch to flip the Slider track gradient when the document direction is set to RTL. This addresses a hardcoded direction in the @rocket.chat/fuselage dependency. Fixes RocketChat#38670
aniruddhaadak80
force-pushed
the
fix/slider-rtl-alignment
branch
from
fd0eaef to
65e9d72
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/meteor/client/lib/utils/injectSliderRTLFix.ts (1)
7-19: UsetextContentinstead ofinnerHTMLfor the style element.
textContentis the semantically correct setter for<style>elements (no HTML parsing; bypasses the innerHTML code path that static analysis flags as CWE-79).♻️ Proposed fix
- styleElement.innerHTML = ` - [dir='rtl'] .rcx-slider-track { - left: auto !important; - right: auto !important; - inset-inline-start: 0 !important; - } - - [dir='rtl'] .rcx-slider-track::before { - transform: scaleX(-1) !important; - transform-origin: center !important; - } - `; + styleElement.textContent = ` + [dir='rtl'] .rcx-slider-track { + left: auto !important; + right: auto !important; + inset-inline-start: 0 !important; + } + + [dir='rtl'] .rcx-slider-track::before { + transform: scaleX(-1) !important; + transform-origin: center !important; + } + `;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/lib/utils/injectSliderRTLFix.ts` around lines 7 - 19, Replace the use of styleElement.innerHTML with styleElement.textContent in injectSliderRTLFix.ts to avoid HTML parsing and CWE-79 flags; locate the style injection block where styleElement and id are set (the template string assigning RTL rules to the style element) and assign that CSS string to styleElement.textContent instead of innerHTML so the <style> content is set semantically and securely.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/meteor/client/lib/utils/injectSliderRTLFix.tsapps/meteor/client/startup/index.tsapps/meteor/client/startup/sliderRTLFix.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/client/startup/index.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/startup/sliderRTLFix.tsapps/meteor/client/lib/utils/injectSliderRTLFix.ts
🧠 Learnings (1)
📚 Learning: 2026-02-10T16:32:42.586Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:42.586Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON and Date fields arrive as Date objects; do not manually construct new Date() in stream handlers (for example, in sdk.stream()). Only REST API responses return plain JSON where dates are strings, so implement explicit conversion there if needed. Apply this guidance to all TypeScript files under apps/meteor/client to ensure consistent date handling in DDP streams and REST responses.
Applied to files:
apps/meteor/client/startup/sliderRTLFix.tsapps/meteor/client/lib/utils/injectSliderRTLFix.ts
🧬 Code graph analysis (1)
apps/meteor/client/startup/sliderRTLFix.ts (1)
apps/meteor/client/lib/utils/injectSliderRTLFix.ts (1)
injectSliderRTLFix(1-21)
🪛 ast-grep (0.40.5)
apps/meteor/client/lib/utils/injectSliderRTLFix.ts
[warning] 7-18: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: styleElement.innerHTML = `
[dir='rtl'] .rcx-slider-track {
left: auto !important;
right: auto !important;
inset-inline-start: 0 !important;
}
[dir='rtl'] .rcx-slider-track::before {
transform: scaleX(-1) !important;
transform-origin: center !important;
}
`
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 7-18: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: styleElement.innerHTML = `
[dir='rtl'] .rcx-slider-track {
left: auto !important;
right: auto !important;
inset-inline-start: 0 !important;
}
[dir='rtl'] .rcx-slider-track::before {
transform: scaleX(-1) !important;
transform-origin: center !important;
}
`
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
🔇 Additional comments (2)
apps/meteor/client/startup/sliderRTLFix.ts (1)
1-7: Startup hook structure is correct.
Meteor.startup()runs after the client DOM is ready, making it a safe point to calldocument.head.appendChild. The hook correctly defers to the utility's own idempotency guard. No issues with this file; the RTL fix correctness concerns are ininjectSliderRTLFix.ts.apps/meteor/client/lib/utils/injectSliderRTLFix.ts (1)
9-13: CSS class.rcx-slider-trackis valid and already in use—revise the verification scope.The
.rcx-slider-trackclass is not speculative; it is actively targeted in the existing RTL fix and confirmed to be emitted by the fuselageSlidercomponent. Your concern that fuselage uses CSS-in-JS with generated class names instead of BEM-style classes appears incorrect; the codebase already applies this fix to the same class.One minor technical point:
right: auto !importanton line 11 is redundant. In RTL context, bothright: autoandinset-inline-start: 0resolve to the physicalrightproperty, and the latter's declaration wins. Theright: autodeclaration has no effect. This is harmless but can be removed.The concerns about non-zero
min/startFromoffsets and range sliders (two thumbs) are theoretical. All Slider usage in the codebase is single-thumb with defaultminValue={0}, and no range slider implementation is present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/meteor/client/lib/utils/injectSliderRTLFix.ts`:
- Around line 15-18: The CSS rule targeting “[dir='rtl']
.rcx-slider-track::before” is dead and the scaleX flip breaks interactions;
remove that rule and instead update injectSliderRTLFix.ts to operate on the
actual DOM element “.rcx-slider-track” (not ::before) and avoid transform flips
— read the element’s inline left/width and swap them into right/width (e.g., set
track.style.right = track.style.left; track.style.left = 'auto') or mutate the
slider render values so the filled segment is positioned via right/width in RTL;
ensure this logic runs for elements under dir='rtl' and on DOM mutations/updates
just like the original injection logic.
---
Nitpick comments:
In `@apps/meteor/client/lib/utils/injectSliderRTLFix.ts`:
- Around line 7-19: Replace the use of styleElement.innerHTML with
styleElement.textContent in injectSliderRTLFix.ts to avoid HTML parsing and
CWE-79 flags; locate the style injection block where styleElement and id are set
(the template string assigning RTL rules to the style element) and assign that
CSS string to styleElement.textContent instead of innerHTML so the <style>
content is set semantically and securely.
2 participants
Proposed changes
This PR implements a global CSS workaround to fix an alignment issue with the
Slidercomponent when the application is in Right-to-Left (RTL) mode.The Problem
In the current version of
@rocket.chat/fuselage, the slider track background (the fill) uses a hardcodedlinear-gradient(to right, ...), causing the highlighted part of the slider to grow from the left even in RTL mode, where it should grow from the right.The Solution
Since the slider logic (powered by
react-aria) correctly mirrors the thumb position, this PR adds a CSS patch that targets theSlidertrack and appliestransform: scaleX(-1)when the document is in RTL mode. This effectively flips the gradient direction without affecting the mirrored interaction logic.Issue(s)
Fixes #38670
Steps to test or reproduce
Summary by CodeRabbit
Bug Fixes
Documentation