UI: Zoom faster when pressing shift#34185
Conversation
Sidnioulz
added
bug
ci:normal
|
View your CI Pipeline Execution ↗ for commit 96340a8
☁️ Nx Cloud last updated this comment at |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a public Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
📝 Coding Plan
Comment Tip You can disable sequence diagrams in the walkthrough.Disable the |
yannbf
left a comment
yannbf
left a comment
There was a problem hiding this comment.
Can you also make the zoom input have that behavior, for consistency sake?
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/core/src/manager/components/preview/NumericInput.tsx`:
- Around line 77-80: NumericInput currently destructures step and shiftStep
(preventing the native HTML step attribute from being forwarded by ...props) and
thus silently blocks the DOM attribute; rename these props to keyboardStep and
keyboardShiftStep throughout the component (update the prop type/interface and
the destructuring in the NumericInput function), use
keyboardStep/keyboardShiftStep for any internal arrow-key logic, and ensure the
original ...props still forwards native HTML attributes (so Form.Input receives
the real step attribute) — update all references to step and shiftStep inside
NumericInput to the new names to avoid shadowing the native attribute.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cf377930-5cbb-46d7-9467-fb7aa9aef1bf
📒 Files selected for processing (2)
code/core/src/manager/components/preview/NumericInput.tsxcode/core/src/manager/components/preview/tools/zoom.tsx
| /** Base step value increase */ | ||
| step?: number; | ||
| /** Step value increase when shift key is pressed */ | ||
| shiftStep?: number; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== NumericInput definition ==="
sed -n '69,205p' code/core/src/manager/components/preview/NumericInput.tsx
echo
echo "=== In-repo NumericInput call sites using step/shiftStep ==="
rg -n -U -P '<NumericInput\b(?:(?!/?>).|\n)*\b(step|shiftStep)\s*=' --glob '*.{ts,tsx,js,jsx}' code
echo
echo "=== Candidates for Form.Input prop typing / native input passthrough ==="
rg -n -C4 'Form\.Input|interface .*InputProps|type .*InputProps|<input\b|step\??:|suffix\??:' --glob '*.{ts,tsx}' codeRepository: storybookjs/storybook
Length of output: 4027
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the Form component
echo "=== Locating Form component ==="
fd -e ts -e tsx 'Form' code --type f | grep -i form | head -20
echo
echo "=== Searching for Form.Input definition or Form component interface ==="
rg -n 'export.*Form|interface Form|const Form|type Form|Form\.Input' code --glob '*.{ts,tsx}' | head -40
echo
echo "=== Looking for form-related component files ==="
fd -type f -name '*form*' code --glob '*.{ts,tsx}' | head -20Repository: storybookjs/storybook
Length of output: 3670
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Form.tsx content ==="
cat -n code/core/src/components/components/Form/Form.tsx
echo
echo "=== Check for Input component import/definition in Form ==="
head -50 code/core/src/components/components/Form/Form.tsxRepository: storybookjs/storybook
Length of output: 1329
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Input.tsx content ==="
cat -n code/core/src/components/components/Form/Input.tsxRepository: storybookjs/storybook
Length of output: 2566
Rename step and shiftStep to avoid shadowing the native HTML step attribute.
Form.Input inherits step from HTMLProps<HTMLInputElement> (not explicitly omitted in its type definition), so it forwards the native HTML step attribute to the underlying input. However, NumericInput now destructures step locally and never forwards it via ...props, which silently prevents the native attribute from reaching the DOM element.
To avoid this semantic ambiguity, rename to keyboardStep and keyboardShiftStep to make it clear these control arrow-key behavior, not native HTML input stepping.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/core/src/manager/components/preview/NumericInput.tsx` around lines 77 -
80, NumericInput currently destructures step and shiftStep (preventing the
native HTML step attribute from being forwarded by ...props) and thus silently
blocks the DOM attribute; rename these props to keyboardStep and
keyboardShiftStep throughout the component (update the prop type/interface and
the destructuring in the NumericInput function), use
keyboardStep/keyboardShiftStep for any internal arrow-key logic, and ensure the
original ...props still forwards native HTML attributes (so Form.Input receives
the real step attribute) — update all references to step and shiftStep inside
NumericInput to the new names to avoid shadowing the native attribute.
3 participants
Fixes an inconsistency we spotted during QA.
Manual testing
Open SB, tab to zoom button, press Shift+ArrowUp and see it zooms by increments of 10% instead of 1%.
Summary by CodeRabbit