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
[WIP] Add export density to canvas renderer #146
base: pixel-density-exports
Are you sure you want to change the base?
Conversation
lucasdinonolte
commented
Sep 2, 2022
- Makes canvas previews crisp on retina displays
- Allows users to choose the desired pixel density before exporting
I was just thinking about this, and I think this is the right implementation. We should make sure that the |
I had the same notion and added some first drafts for error messages. https://github.com/designsystemsinternational/mechanic/pull/146/files#diff-cea56d01b8f19fd6436a19f2b4bbd9d2258c6edbeef3df917df2026fcf1ed520R82 😊 |
state: mechanic.functionState, | ||
setState: onSetState, | ||
}, | ||
getCanvas: ({ width = null, height = null } = {}) => { |
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.
I think it's a bit odd from the interface that we ask for the canvas and context from this function and then are supposed to give it back through frame
and done
. Would it make sense to keep a reference inside the engine and then use it when frame and done get called?
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.
I like that idea!
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.
Oh, I like it. I tried it and it's very nice. The only situation this is problematic is, when you also want to pass a name
to onDone
. So I changed done
to accept an object as arguments. Let me know what you think.
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.
I do like it! It does made me realize there's no mention in the documentation about the name argument for done
. Maybe this is an opportunity to streamline arguments for the frame
and done
functions and leave all of them as single argument functions on all engines. Can be done in a separate PR tho.
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.
I'm all in for this!
Co-authored-by: Fernando Alberto Florenzano Hernández <f.florenzano94@gmail.com>