Zoom: Keyboardshortcut for the plus key#33565
Conversation
ndelangen
added
feature request
a11y: zoom
|
View your CI Pipeline Execution ↗ for commit 6587802
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughAdds a second keyboard shortcut (Alt+Plus) for the zoom-in action in the zoom tool, registered alongside the existing Alt+= mapping. No behavioral or control-flow changes were made. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (2)
✏️ Tip: You can disable this entire section by setting 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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/core/src/manager/components/preview/tools/zoom.tsx (1)
193-204: Use uniqueactionNamevalues for the two zoom-in shortcuts; currently both use'zoomIn'and the second registration will overwrite the first.Based on the shortcut registration pattern (key format:
${addon}-${actionName}), both calls withactionName: 'zoomIn'create the same keyzoom-zoomIn, causing the second call to overwrite the first. This means only the['alt', '+']shortcut will be functional;['alt', '=']will be ignored.Rename one to
'zoomInEquals'and the other to'zoomInPlus'to ensure both shortcuts are registered independently.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/core/src/manager/components/preview/tools/zoom.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/core/src/manager/components/preview/tools/zoom.tsx
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/core/src/manager/components/preview/tools/zoom.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/core/src/manager/components/preview/tools/zoom.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not useconsole.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/manager/components/preview/tools/zoom.tsx
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
loggerfromstorybook/internal/node-loggerfor server-side logging in Node.js code
Files:
code/core/src/manager/components/preview/tools/zoom.tsx
⏰ 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). (4)
- GitHub Check: normal-generated
- GitHub Check: nx
- GitHub Check: get-parameters
- GitHub Check: get-branch
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 183 | 183 | 0 |
| Self size | 775 KB | 775 KB | 🚨 +139 B 🚨 |
| Dependency size | 67.38 MB | 67.45 MB | 🚨 +68 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 176 | 176 | 0 |
| Self size | 30 KB | 30 KB | 🎉 -4 B 🎉 |
| Dependency size | 65.95 MB | 66.02 MB | 🚨 +68 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
| api.setAddonShortcut('zoom', { | ||
| label: 'Zoom in', | ||
| defaultShortcut: ['alt', '+'], | ||
| actionName: 'zoomIn', | ||
| action: zoomIn, | ||
| }); |
There was a problem hiding this comment.
Did you verify this works? I would thing this is going to override the existing zoomIn action. You should probably give it an alternative name, such as zoomInAlt. Do not that it will then show up twice in the keyboard shortcuts table.
There was a problem hiding this comment.
I THINK it tried both, but now you're making me second guess myself, I'll re-test it locally!
Yup, good catch! I'll change it, so they both work.
I tried looking for a way for a single "action" to be linked to multiple keyboard-shortcuts, but that's not allowed by the system we have, and this is not the right time to be changing that significantly.
shilman
added
the
a11y: keyboard
3 participants
What I did
Added an additional zoom in keyboard shortcut
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
I manually verified this works on my local machine using my own keyboard
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.