XS-13706 | XS-13744 [Merch Improvement]: Add Tax section on Pricing & Availability page - #414
KarthickXola wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Replaces a plain <div> with <Dialog.Panel> from Headless UI in the Drawer component. This scopes the click-outside-to-close behavior properly to the panel content.
Changes:
- Swap inner
<div>for<Dialog.Panel>in the Drawer component for correct Headless UI dialog semantics
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {position === "right" ? <CloseButton onClose={onClose} /> : null} | ||
|
|
||
| <div | ||
| <Dialog.Panel |
Summary
Walkthrough
ChangesDrawer interaction handling
Merge Risk: 🔴 Critical · up to The Drawer can currently fail to render because useRef is not imported; even when it renders, titles are missing and close controls do not dismiss the drawer. These are release-blocking correctness regressions that should be fixed before merging, alongside smaller debug-output and formatting cleanup. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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: 7
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5f4330e5-4642-4b69-aa4e-00a85e285ebe
📒 Files selected for processing (1)
src/components/Drawer.jsx
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
xola/x2-seller(manual)
Included review availability: 0 reviews are currently available. Based on recent review activity, included reviews refill at 5 per hour.
| const shouldIgnoreCloseRef = useRef(false); | ||
| const resetCloseGuardTimeoutRef = useRef(null); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 2 'useRef|from .*react' src/components/Drawer.jsxRepository: xola/ui-kit
Length of output: 667
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,90p' src/components/Drawer.jsx
printf '\nReact import and useRef occurrences:\n'
rg -n '^(import .*react|.*useRef)' src/components/Drawer.jsx
printf '\nPackage/lint context:\n'
rg -n '"(react|eslint|eslint-plugin-react-hooks)"|react-hooks|no-undef' package.json .eslintrc* 2>/dev/null || trueRepository: xola/ui-kit
Length of output: 4245
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
path = Path("src/components/Drawer.jsx")
source = path.read_text()
imports = re.findall(r'import\s+([^;]+?)\s+from\s+["\']react["\']', source)
uses_ref = bool(re.search(r'\buseRef\s*\(', source))
has_ref_binding = any(re.search(r'\buseRef\b', item) for item in imports)
print({
"uses_useRef": uses_ref,
"react_imports": imports,
"useRef_bound_by_react_import": has_ref_binding,
"unbound_useRef": uses_ref and not has_ref_binding,
})
PYRepository: xola/ui-kit
Length of output: 288
Import useRef from React. Drawer calls useRef, but the React import does not bind it. Every render throws ReferenceError: useRef is not defined.
🧰 Tools
🪛 GitHub Check: View Lint Report
[failure] 35-35: src/components/Drawer.jsx#L35
[prettier/prettier] Delete ·
[failure] 35-35: src/components/Drawer.jsx#L35
[no-undef] 'useRef' is not defined.
[failure] 36-36: src/components/Drawer.jsx#L36
[prettier/prettier] Delete ·
[failure] 36-36: src/components/Drawer.jsx#L36
[no-undef] 'useRef' is not defined.
Source: Linters/SAST tools
| const shouldIgnoreCloseRef = useRef(false); | ||
| const resetCloseGuardTimeoutRef = useRef(null); | ||
|
|
||
| const handleContentInteraction = () => { | ||
| shouldIgnoreCloseRef.current = true; | ||
| if (resetCloseGuardTimeoutRef.current) { | ||
| window.clearTimeout(resetCloseGuardTimeoutRef.current); | ||
| } | ||
| resetCloseGuardTimeoutRef.current = window.setTimeout(() => { | ||
| shouldIgnoreCloseRef.current = false; | ||
| }, 0); | ||
| }; | ||
|
|
||
| const handleClose = () => { | ||
| if (shouldIgnoreCloseRef.current) { | ||
| shouldIgnoreCloseRef.current = false; | ||
| return; | ||
| } | ||
| onClose(); | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the whitespace that fails Prettier.
Prettier reports trailing whitespace in Lines 35-54 and whitespace-only content before the panel content on Lines 111-112. Remove the extra spaces.
Also applies to: 111-112
🧰 Tools
🪛 GitHub Check: View Lint Report
[failure] 35-35: src/components/Drawer.jsx#L35
[prettier/prettier] Delete ·
[failure] 35-35: src/components/Drawer.jsx#L35
[no-undef] 'useRef' is not defined.
[failure] 36-36: src/components/Drawer.jsx#L36
[prettier/prettier] Delete ·
[failure] 36-36: src/components/Drawer.jsx#L36
[no-undef] 'useRef' is not defined.
[failure] 38-38: src/components/Drawer.jsx#L38
[prettier/prettier] Delete ·
[failure] 39-39: src/components/Drawer.jsx#L39
[prettier/prettier] Delete ·
[failure] 40-40: src/components/Drawer.jsx#L40
[prettier/prettier] Delete ·
[failure] 41-41: src/components/Drawer.jsx#L41
[prettier/prettier] Delete ·
[failure] 42-42: src/components/Drawer.jsx#L42
[prettier/prettier] Delete ·
[failure] 43-43: src/components/Drawer.jsx#L43
[prettier/prettier] Delete ·
[failure] 44-44: src/components/Drawer.jsx#L44
[prettier/prettier] Delete ·
[failure] 45-45: src/components/Drawer.jsx#L45
[prettier/prettier] Delete ·
[failure] 46-46: src/components/Drawer.jsx#L46
[prettier/prettier] Delete ·
[failure] 48-48: src/components/Drawer.jsx#L48
[prettier/prettier] Delete ·
[failure] 49-49: src/components/Drawer.jsx#L49
[prettier/prettier] Delete ·
[failure] 50-50: src/components/Drawer.jsx#L50
[prettier/prettier] Delete ·
[failure] 51-51: src/components/Drawer.jsx#L51
[prettier/prettier] Delete ·
[failure] 52-52: src/components/Drawer.jsx#L52
[prettier/prettier] Delete ·
[failure] 53-53: src/components/Drawer.jsx#L53
[prettier/prettier] Delete ·
[failure] 54-54: src/components/Drawer.jsx#L54
[prettier/prettier] Delete ·
Source: Linters/SAST tools
| const handleClose = () => { | ||
| if (shouldIgnoreCloseRef.current) { | ||
| shouldIgnoreCloseRef.current = false; | ||
| return; | ||
| } | ||
| onClose(); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Drawer.jsx ---'
cat -n src/components/Drawer.jsx
printf '%s\n' '--- Sidebar consumers ---'
rg -n -A12 -B8 'onClose|Drawer|CloseButton' src/components/Sidebar src/components 2>/dev/null | head -n 240
printf '%s\n' '--- close-button definitions/usages ---'
rg -n -A10 -B8 'CloseButton' src
printf '%s\n' '--- relevant dependency versions ---'
rg -n '"`@headlessui/react`"|"react"|"ahooks"' package.json yarn.lock package-lock.json pnpm-lock.yaml 2>/dev/null | head -n 80
printf '%s\n' '--- Drawer stories ---'
rg -n -A12 -B5 'Drawer' src/stories src 2>/dev/null | head -n 180Repository: xola/ui-kit
Length of output: 49425
🏁 Script executed:
#!/bin/bash
set -eu
cat -n src/components/Drawer.jsx
printf '\n--- Sidebar close consumers ---\n'
rg -n -A12 -B8 'onClose|Drawer|CloseButton' src/components/Sidebar src/components 2>/dev/null | head -n 240
printf '\n--- CloseButton definitions/usages ---\n'
rg -n -A10 -B8 'CloseButton' src
printf '\n--- dependency versions ---\n'
rg -n '"`@headlessui/react`"|"react"|"ahooks"' package.json yarn.lock package-lock.json pnpm-lock.yaml 2>/dev/null | head -n 80
printf '\n--- Drawer stories ---\n'
rg -n -A12 -B5 'Drawer' src/stories src 2>/dev/null | head -n 180Repository: xola/ui-kit
Length of output: 49753
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Button implementation ---'
fd -t f -i 'Button*.jsx' src/components | xargs -r -n1 sh -c 'echo "--- $0"; cat -n "$0"' | head -n 260
printf '%s\n' '--- exact onClick forwarding ---'
rg -n -A18 -B12 'onClick' src/components/Buttons src/components/Button* 2>/dev/null | head -n 220
printf '%s\n' '--- standalone callback-contract check ---'
python3 - <<'PY'
from pathlib import Path
drawer = Path("src/components/Drawer.jsx").read_text()
sidebar = Path("src/components/Sidebar/Sidebar.jsx").read_text()
checks = {
"CloseButton receives handleClose": "<CloseButton onClose={handleClose} />" in drawer,
"CloseButton passes onClick to Button": "onClick={onClose}" in drawer,
"handleClose has no parameter": "const handleClose = () =>" in drawer,
"handleClose calls onClose without argument": "onClose();" in drawer,
"Sidebar requires truthy close argument": "onClose={(e) => !!e && handleDrawerStateChange" in sidebar,
}
for name, result in checks.items():
print(f"{name}: {result}")
PYRepository: xola/ui-kit
Length of output: 16509
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Button implementation ---'
fd -t f -i 'Button*.jsx' src/components | xargs -r -n1 sh -c 'echo "--- $0"; cat -n "$0"' | head -n 260
printf '%s\n' '--- exact onClick forwarding ---'
rg -n -A18 -B12 'onClick' src/components/Buttons src/components/Button* 2>/dev/null | head -n 220
printf '%s\n' '--- callback-contract check ---'
python3 - <<'PY'
from pathlib import Path
drawer = Path("src/components/Drawer.jsx").read_text()
sidebar = Path("src/components/Sidebar/Sidebar.jsx").read_text()
checks = {
"CloseButton receives handleClose": "<CloseButton onClose={handleClose} />" in drawer,
"CloseButton passes onClick to Button": "onClick={onClose}" in drawer,
"handleClose has no parameter": "const handleClose = () =>" in drawer,
"handleClose calls onClose without argument": "onClose();" in drawer,
"Sidebar requires truthy close argument": "onClose={(e) => !!e && handleDrawerStateChange" in sidebar,
}
for name, result in checks.items():
print(f"{name}: {result}")
PYRepository: xola/ui-kit
Length of output: 16498
Forward the close callback argument.
Button forwards the click event to handleClose, but handleClose calls onClose() without it. Sidebar ignores falsy close arguments, so both drawer close buttons do nothing.
-const handleClose = () => {
+const handleClose = (closeArgument) => {
...
- onClose();
+ onClose(closeArgument);📝 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.
| const handleClose = () => { | |
| if (shouldIgnoreCloseRef.current) { | |
| shouldIgnoreCloseRef.current = false; | |
| return; | |
| } | |
| onClose(); | |
| }; | |
| const handleClose = (closeArgument) => { | |
| if (shouldIgnoreCloseRef.current) { | |
| shouldIgnoreCloseRef.current = false; | |
| return; | |
| } | |
| onClose(closeArgument); | |
| }; |
🧰 Tools
🪛 GitHub Check: View Lint Report
[failure] 48-48: src/components/Drawer.jsx#L48
[prettier/prettier] Delete ·
[failure] 49-49: src/components/Drawer.jsx#L49
[prettier/prettier] Delete ·
[failure] 50-50: src/components/Drawer.jsx#L50
[prettier/prettier] Delete ·
[failure] 51-51: src/components/Drawer.jsx#L51
[prettier/prettier] Delete ·
[failure] 52-52: src/components/Drawer.jsx#L52
[prettier/prettier] Delete ·
[failure] 53-53: src/components/Drawer.jsx#L53
[prettier/prettier] Delete ·
[failure] 54-54: src/components/Drawer.jsx#L54
[prettier/prettier] Delete ·
| } | ||
| onClose(); | ||
| }; | ||
| console.log("Drawer rendered with isOpen:", isOpen, "position:", position, "sideIndent:", sideIndent); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Remove the render-time debug log.
console.log runs on every render and pollutes production output. Remove it before merge.
| {position === "right" ? <CloseButton onClose={handleClose} /> : null} | ||
| <Dialog.Panel | ||
| className={clsx( | ||
| "flex h-full w-full flex-col overflow-y-auto bg-white px-4 py-8 shadow-xl sm:px-6", | ||
| sizes[size], | ||
| classNames.children, | ||
| )} | ||
| onMouseDownCapture={handleContentInteraction} | ||
| onTouchStartCapture={handleContentInteraction} | ||
| > | ||
| <div className="w-full"> | ||
| {/* eslint-disable-next-line react/jsx-max-depth */} | ||
| <Dialog.Title>{title}</Dialog.Title> | ||
| </div> | ||
|
|
||
| <div className={clsx("relative mt-3 flex-1", classNames.content)}> | ||
| {"testUIKIT1"} | ||
| {content} | ||
| </div> | ||
| </div> | ||
| </Dialog.Panel> | ||
|
|
||
| {position === "left" ? <CloseButton onClose={onClose} /> : null} | ||
| {position === "left" ? <CloseButton onClose={handleClose} /> : null} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 '\bDrawer\b' src/storiesRepository: xola/ui-kit
Length of output: 3079
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Drawer story ---'
cat -n src/stories/Overlay/Drawer.stories.jsx
printf '%s\n' '--- Drawer component ---'
cat -n src/components/Drawer.jsx
printf '%s\n' '--- Story history/status ---'
git status --short
git diff --statRepository: xola/ui-kit
Length of output: 10141
Add Drawer Storybook coverage for all close paths.
Update src/stories/Overlay/Drawer.stories.jsx to cover panel interaction, both close-button positions, and the left default. Do not add a rendered Jest test.
🧰 Tools
🪛 GitHub Check: View Lint Report
[failure] 111-112: src/components/Drawer.jsx#L111-L112
[prettier/prettier] Delete ⏎······································
[failure] 114-114: src/components/Drawer.jsx#L114
[react/jsx-curly-brace-presence] Curly braces are unnecessary here.
Source: Coding guidelines
| <div className={clsx("relative mt-3 flex-1", classNames.content)}> | ||
| {"testUIKIT1"} | ||
| {content} | ||
| </div> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restore the title in the rendered panel.
title remains a public prop, and src/components/Sidebar/Sidebar.jsx passes title={leftDrawer.title} and title={rightDrawer.title}. The new content block renders only the marker and content, so both drawer titles disappear. Render title again before content.
🧰 Tools
🪛 GitHub Check: View Lint Report
[failure] 114-114: src/components/Drawer.jsx#L114
[react/jsx-curly-brace-presence] Curly braces are unnecessary here.
| </div> | ||
|
|
||
| <div className={clsx("relative mt-3 flex-1", classNames.content)}> | ||
| {"testUIKIT1"} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not render testUIKIT1 as user-facing text.
If testUIKIT1 is a test hook, attach it to a data-testid attribute. Otherwise, remove it. The current JSX inserts the literal marker into every Drawer and triggers react/jsx-curly-brace-presence.
🧰 Tools
🪛 GitHub Check: View Lint Report
[failure] 114-114: src/components/Drawer.jsx#L114
[react/jsx-curly-brace-presence] Curly braces are unnecessary here.
Source: Linters/SAST tools
No description provided.