-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Add DPI Converter extension #24355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add DPI Converter extension #24355
Conversation
|
Congratulations on your new Raycast extension! 🚀 Due to our current reduced availability, the initial review may take up to 10-15 business days. Once the PR is approved and merged, the extension will be available on our Store. |
Greptile OverviewGreptile SummaryThis PR adds a new DPI Converter extension that converts between pixels, centimeters, millimeters, and inches with customizable DPI support. The extension provides both single and dual dimension conversion capabilities with detailed step-by-step calculations. Key Features:
Implementation Quality:
The extension includes proper metadata screenshots and follows the standard extension structure. Overall, this is a useful utility extension with solid functionality that just needs minor fixes to the validation logic. Confidence Score: 4/5
Important Files ChangedFile Analysis
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 files reviewed, 4 comments
| } | ||
|
|
||
| async function handleSubmit(values: { dimension1: string; dimension2?: string; dpi: string }) { | ||
| const dpiValue = parseInt(values.dpi) || 300; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The DPI validation here doesn't handle negative numbers. When parseInt(values.dpi) returns a negative number like -10, the || 300 fallback won't trigger because -10 is truthy. This will cause incorrect calculations with negative DPI values.
Consider adding proper validation:
| const dpiValue = parseInt(values.dpi) || 300; | |
| const dpiValue = Math.max(1, parseInt(values.dpi) || 300); |
This ensures DPI is always at least 1, preventing division by negative numbers or very small values that could cause precision issues.
| ], | ||
| "dependencies": { | ||
| "@raycast/api": "^1.104.1", | ||
| "@raycast/utils": "^1.17.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raycast/utils is listed as a dependency but is never imported or used anywhere in the extension code. This adds unnecessary weight to the bundle.
| "@raycast/utils": "^1.17.0" | |
| "dependencies": { | |
| "@raycast/api": "^1.104.1" | |
| }, |
Remove this unused dependency to reduce bundle size.
| "$schema": "https://www.raycast.com/schemas/extension.json", | ||
| "name": "dpi-converter", | ||
| "title": "DPI Converter", | ||
| "description": "Convert pixels in centimeters, millimeter and inches, and vice-versa! Useful for printers.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The description has grammatical errors: "Convert pixels in centimeters, millimeter and inches" should use "to" instead of "in", "millimeters" should be plural, and there should be a comma before "and".
| "description": "Convert pixels in centimeters, millimeter and inches, and vice-versa! Useful for printers.", | |
| "description": "Convert pixels to centimeters, millimeters, and inches, and vice-versa! Useful for printers.", |
| { | ||
| "name": "convert-to-px-cm-mm-in", | ||
| "title": "Convert to px, cm, mm, in", | ||
| "description": "Convert px in cm, mm, in, and vice-versa.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same grammatical issue as the main description: "Convert px in cm" should be "Convert px to cm".
| "description": "Convert px in cm, mm, in, and vice-versa.", | |
| "description": "Convert px to cm, mm, in, and vice-versa.", |
0xdhrv
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution @Qualigraphe 🔥
We already have core calculator which deals with this.
If there are unique features or workflows you’re aiming to add, we’d love to hear them as feedback and see if they can be integrated into one of these to avoid duplication and improve discoverability.
This would help avoid duplication and keep related functionality consolidated in one place.
As mentioned in our extension guidelines here ↗
|
Hi @0xdhrv, Thank you so much for taking the time to review my contribution and for your feedback! 🙏 I really appreciate the thoughtful response and completely understand the concern about avoiding duplication. I'd love to clarify why I believe DPI Converter addresses a specific workflow gap that Calculator doesn't currently cover, based on my daily work with designers and print professionals. Real-World Use CasesAs a designer and trainer working daily with print professionals, these are critical daily workflows: Example 1: Large Format Print
Example 2: Video Screenshot to Print
These examples demonstrate why DPI context is essential – you need different pixel dimensions based on viewing distance and print technology, and you need to know printable dimensions from existing pixel assets. Why This is Unique to DPI ConverterThis is a unique capability that Calculator does not currently offer. Calculator handles standard unit conversions excellently, but it has no mechanism to perform resolution-dependent conversions where the relationship between pixels and physical units depends on a third parameter (DPI). Queries like those in the example above simply cannot be resolved without knowing the target resolution. Potential Integration into CalculatorIf Calculator could support this syntax natively, that would be ideal. Maybe by typing something like this (though I'm concerned it might become too complicated for the syntax):
If no DPI is specified, defaulting to 300 DPI makes sense because it's the universal standard for commercial printers, photo labs, and professional print houses.
|
Description
DPI Converter is a utility extension that converts between pixels, centimeters, millimeters, and inches with customizable DPI support.
Features:
Use Cases:
Perfect for designers, publications editors and printing houses who need to convert units for print design (A4, US Letter…). This extension allows them, for example, to know how many pixels are the minimum required to print at a good resolution.
Screencast
Screenshots are included in the
metadata/folder showing:Checklist
npm run buildand tested this distribution build in Raycastassetsfolder are used by the extension itselfREADMEare located outside the metadata folder if they were not generated with our metadata