Code Review
Closed this issue · 6 comments
- https://github.com/skpm/print-export-sketchplugin/blob/master/src/utils.js#L54-L67 should be
require('path').baseName - instead of having a custom nib loader, we should try to use https://github.com/skpm/nib-loader
- Could you quickly explain why we need an obj-c framework?
- Instead of "Continue" as the label of the button, it should probably be something more explicit like "Export pdf" or something
instead of having a custom nib loader, we should try to use https://github.com/skpm/nib-loader
I looked at it but it doesn't make sense in this situation because the nib is already part of the framework that's bundled with the plugin.
Could you quickly explain why we need an obj-c framework?
Because I'm using Core Graphics, it uses some C which I can't bridge to JavaScript.
Instead of "Continue" as the label of the button, it should probably be something more explicit like "Export pdf" or something
I had it labeled "Export" but when I was writing the instructions it seemed weird to say press Export twice - the 2nd time in the save file dialog. I might change it back.
because the nib is already part of the framework that's bundled with the plugin.
Still, I don't think it would change anything? The code looks exactly the same except that bundleWithURL is bundleWithIdentifier
I'm using Core Graphics
That's not a reason tho. There are pure JS solution to generate a pdf (https://github.com/MrRio/jsPDF for example)
Still, I don't think it would change anything? The code looks exactly the same except that bundleWithURL is bundleWithIdentifier
Your library accesses the plugin bundle, I need to access the framework bundle.
That's not a reason tho. There are pure JS solution to generate a pdf (https://github.com/MrRio/jsPDF for example)
I looked into them but felt they wouldn't be up to the job of creating a PDF/X. Also jsPDF doesn't support shadows (at least shadows rendered in the CMYK colorspace).
Your library accesses the plugin bundle, I need to access the framework bundle.
Ok sorry, the question is, why does it need to be bundled in the framework? Does the framework use it?
Ok sorry, the question is, why does it need to be bundled in the framework? Does the framework use it?
I could compile it separately but it's more natural to have it part of the framework. How about providing an option to load a nib from any bundle? The dev provides a bundle name and assume it's in the Resources folder of the plugin.
It's a lot easier to do require('../path/to/xib') tho, the compilation is done for you, etc.
I don't get why it's more natural to have it part of the framework.