๐จ Palette: Add dynamic human-readable size preview - #58
Conversation
Adds a dynamic, ARIA-announced preview to the target bytes input in saas_web.py to prevent magnitude errors.
|
๐ Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a ๐ emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Warning Review limit reached
More reviews will be available in 58 minutes and 3 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. โ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. ๐ฆ How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. โน๏ธ Review infoโ๏ธ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ๐ Files selected for processing (2)
๐ WalkthroughWalkthroughํ์ผ ํฌ๊ธฐ ์ ๋ ฅ ํ๋์ ์ฌ์ฉ์ ์นํ์ ์ธ ์ค์๊ฐ ๋ฏธ๋ฆฌ๋ณด๊ธฐ๋ฅผ ์ถ๊ฐํ๋ ์ ๊ทผ์ฑ ๊ธฐ๋ฅ ๊ตฌํ. ๊ฐ์ด๋๋ผ์ธ ๋ฌธ์ํ, HTML/JavaScript ๊ตฌํ, ์ ๊ทผ์ฑ ์์ฑ ๊ฒ์ฆ์ ํฌํจ. ChangesAccessible File Size Preview
Sequence Diagram(s)sequenceDiagram
participant User
participant Input as target_bytes input
participant Handler as oninput handler
participant Preview as target_bytes_preview element
participant Form as shrink-form submit
participant Button as submit button
User->>Input: ํฌ๊ธฐ ๊ฐ ์
๋ ฅ
Input->>Handler: oninput ์ด๋ฒคํธ ๋ฐ์
Handler->>Handler: ๋ฐ์ดํธ ๊ฐ ๊ฒ์ฆ
Handler->>Handler: B/KB/MB/GB ๋จ์ ๋ณํ
Handler->>Preview: ๋ณํ๋ ํฌ๊ธฐ ํ์ ๊ฐฑ์
Preview->>Preview: aria-live="polite" ์์ญ์ผ๋ก ์๋ฆผ
User->>Form: ํผ ์ ์ถ
Form->>Button: aria-busy='true' ์ค์
Form->>Button: ๋ฒํผ ๋นํ์ฑํ
Estimated code review effort๐ฏ 2 (Simple) | โฑ๏ธ ~12 minutes Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
โ Failed checks (1 error)
โ Passed checks (4 passed)
โจ Finishing Touches๐งช Generate unit tests (beta)
โจ Simplify code
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
๐งน Nitpick comments (2)
saas_web.py (1)
39-39: ๐ค Low valueํ์ผ ํฌ๊ธฐ ๋จ์ ๋ณํ ๊ธฐ์ค์ ๋ช ํํ ํ๋ ๊ฒ์ ๊ณ ๋ คํ์ธ์.
ํ์ฌ ๊ตฌํ์ SI ๋จ์(1000 ๊ธฐ๋ฐ)๋ฅผ ์ฌ์ฉํฉ๋๋ค:
- 1 KB = 1,000 bytes
- 1 MB = 1,000,000 bytes
- 1 GB = 1,000,000,000 bytes
์ด๋ ๊ธฐ์ ์ ์ผ๋ก ์ ํํ์ง๋ง, ๋ง์ ์ฌ์ฉ์๋ค์ ์ด์ง ๋จ์(1024 ๊ธฐ๋ฐ: KiB, MiB, GiB)์ ์ต์ํฉ๋๋ค. ์๋ฅผ ๋ค์ด, 2,000,000,000 ๋ฐ์ดํธ๋:
- SI ๋จ์: 2.00 GB
- ์ด์ง ๋จ์: 1.86 GiB
ํ์ฌ ๊ตฌํ์ ์ ์งํ ๊ฒฝ์ฐ, ๋จ์ ๋ ์ด๋ธ์ "GB (decimal)" ๋๋ "GB"๋ก ๋ช ์ํ๊ฑฐ๋, ์ด์ง ๋จ์๋ก ์ ํํ๋ ๊ฒ์ ๊ณ ๋ คํ์ธ์.
๐ค 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 `@saas_web.py` at line 39, The byte-to-unit conversion in the oninput handler for the target_bytes input (id="target_bytes", preview element id="target_bytes_preview") currently uses 1000-based SI units; either explicitly label the preview with "GB (decimal)"/"MB (decimal)" or change the conversion to 1024-based binary units (KiB/MiB/GiB) โ implement by updating the oninput logic that computes preview.innerText to use either 1000 or 1024 multipliers and corresponding unit strings, or add a small toggle control that switches a mode variable between "decimal" and "binary" and branches the conversion accordingly so the preview shows the chosen unit system (e.g., "1.86 GiB" vs "2.00 GB").tests/test_saas_web.py (1)
28-37: โก Quick winํ ์คํธ ์ด์ค์ ์ ๊ฒฌ๊ณ ์ฑ์ ๊ฐ์ ํ์ธ์.
Line 37์ ์ด์ค์ ์ JavaScript ์ฝ๋์ ์ ํํ ๋ฌธ์์ด์ ํ์ธํฉ๋๋ค:
self.assertIn("btn.setAttribute('aria-busy', 'true')", html)์ด๋ ์ทจ์ฝํฉ๋๋ค:
- ๊ณต๋ฐฑ ๋ณ๊ฒฝ (์:
setAttribute( 'aria-busy')- ๋ฐ์ดํ ์คํ์ผ ๋ณ๊ฒฝ (์:
"aria-busy"๋์'aria-busy')- ์ฝ๋ ํฌ๋งทํฐ ์ ์ฉ
์ ํ ์คํธ๊ฐ ์คํจํฉ๋๋ค.
๋ ๊ฒฌ๊ณ ํ ๋์:
- ์ ๊ท์ ์ฌ์ฉ:
assertRegex(html, r"setAttribute\(['\"]aria-busy['\"],\s*['\"]true['\"]")- ๋๋ ํต์ฌ ๊ฐ๋ ๋ง ํ์ธ:
assertIn('aria-busy', html)(์ถฉ๋ถํ ์ ์์)โป๏ธ ๋ ๊ฒฌ๊ณ ํ ์ด์ค์ ์ ์
self.assertIn('aria-describedby="target_bytes_help target_bytes_preview"', html) self.assertIn('id="target_bytes_preview"', html) self.assertIn('aria-live="polite"', html) self.assertIn(">2.00 GB</span>", html) - self.assertIn("btn.setAttribute('aria-busy', 'true')", html) + # aria-busy ์์ฑ์ด ์ค์ ๋๋์ง ํ์ธ (์ ํํ JavaScript ๊ตฌ๋ฌธ์ ๋ ์์กด) + self.assertIn('aria-busy', html) + self.assertIn('submit-btn', html)๐ค 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 `@tests/test_saas_web.py` around lines 28 - 37, Update the fragile assertion in test_get_ui_includes_accessible_target_size_preview: instead of asserting the exact JS substring "btn.setAttribute('aria-busy', 'true')", make the check resilient to spacing and quote changes by using a regex like setAttribute\(['"]aria-busy['"],\s*['"]true['"]\) with assertRegex, or simplify to assertIn('aria-busy', html) if only presence is required; modify the assertion referencing the test method and the JS occurrence (btn.setAttribute / aria-busy) accordingly.
๐ค 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 `@saas_web.py`:
- Line 39: Move the long inline oninput handler for the input with id
"target_bytes" into a separate <script> block: extract the logic into a named
function (e.g., updateTargetBytesPreview) that reads the element value, parses
it with parseInt, handles NaN/<=0, and formats bytes-to-B/KB/MB/GB into the
element with id "target_bytes_preview"; then remove the inline oninput attribute
and attach the function via addEventListener('input', updateTargetBytesPreview)
on the target_bytes element (also call it once on load to initialize the
preview). Ensure the function name and DOM ids ("target_bytes",
"target_bytes_preview") from the diff are used so the handler targets the
correct elements and keep validation (min/required) intact.
- Around line 45-54: Replace the setTimeout-based submit handler for the form
with a synchronous guard: in the 'shrink-form' submit event handler, immediately
get the 'submit-btn' and if it's already disabled, prevent the submission and
return; otherwise synchronously disable the button, set its innerText to
'Processing...' and aria-busy to 'true' before allowing the form to submit. This
removes the setTimeout race and ensures double submissions are blocked by
checking/disabling 'submit-btn' at the start of the handler.
---
Nitpick comments:
In `@saas_web.py`:
- Line 39: The byte-to-unit conversion in the oninput handler for the
target_bytes input (id="target_bytes", preview element
id="target_bytes_preview") currently uses 1000-based SI units; either explicitly
label the preview with "GB (decimal)"/"MB (decimal)" or change the conversion to
1024-based binary units (KiB/MiB/GiB) โ implement by updating the oninput logic
that computes preview.innerText to use either 1000 or 1024 multipliers and
corresponding unit strings, or add a small toggle control that switches a mode
variable between "decimal" and "binary" and branches the conversion accordingly
so the preview shows the chosen unit system (e.g., "1.86 GiB" vs "2.00 GB").
In `@tests/test_saas_web.py`:
- Around line 28-37: Update the fragile assertion in
test_get_ui_includes_accessible_target_size_preview: instead of asserting the
exact JS substring "btn.setAttribute('aria-busy', 'true')", make the check
resilient to spacing and quote changes by using a regex like
setAttribute\(['"]aria-busy['"],\s*['"]true['"]\) with assertRegex, or simplify
to assertIn('aria-busy', html) if only presence is required; modify the
assertion referencing the test method and the JS occurrence (btn.setAttribute /
aria-busy) accordingly.
๐ช 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 972db1d2-dfe0-43d1-8cd9-f3631c3044c5
๐ Files selected for processing (3)
.jules/palette.mdsaas_web.pytests/test_saas_web.py
|
@coderabbitai review |
โ Action performedReview finished.
|
Adds a dynamic, ARIA-announced preview to the target bytes input in saas_web.py to prevent magnitude errors.
๐ก What
Added a dynamic human-readable file size preview (e.g., "2.00 GB", "500.00 MB") right next to the "Target Bytes" input in
saas_web.py. The preview updates automatically as the user types using inline JavaScript.๐ฏ Why
Users often struggle to conceptualize large raw byte numbers (like
2000000000), which easily leads to magnitude errors (e.g., adding an extra zero). Providing a live preview in familiar units (MB, GB) drastically improves user confidence and prevents submission mistakes.๐ธ Before/After
Before: Users just see a text box containing
2000000000.After: Users see a green, bold preview text below the input that says
2.00 GB, which dynamically updates as they edit the input.โฟ Accessibility
aria-describedbyattribute.<span>hasaria-live="polite"so screen readers will announce the calculated size as the user finishes typing.aria-busy="true"when clicked, signaling to screen readers that processing is happening.PR created automatically by Jules for task 8836426316054162301 started by @seonghobae