-
Notifications
You must be signed in to change notification settings - Fork 185
Use getFilePreview
for screenshots in Console
#2140
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
Use getFilePreview
for screenshots in Console
#2140
Conversation
.storage.getFilePreview( | ||
'screenshots', | ||
fileId, | ||
undefined, |
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.
It'd have been great if this method accepted an object
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.
Its in our backlog :(
return sdk.forConsoleIn(page.params.region).storage.getFileView('screenshots', fileId); | ||
return sdk | ||
.forConsoleIn(page.params.region) | ||
.storage.getFilePreview( |
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.
Let's also set resolutions for image, something reasonable. See what size image has on big screen, and double that (for retina displays). such size should be enough to have all details.
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.
If you have macos with retina, double-check if it looks blurry or detailed. You can also ask me if you want to double check. If it looks blurry, let's stay at source resolution - not override it
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 wouldn't worry too much about the resolutions. a fixed, all in one, should be fine if we use a correct image format type. I expect previews should be cached as well. a mid range resolution should be just fine.
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.
Update the resolution to 1024 x 576 (looks good, imo).
Marking this as draft since the backend PR is WIP. cc @Meldiron |
@ItzNotABug backend PR has been merged. Could you please review this PR? |
What does this PR do?
file-preview
API to loadwebp
images shown in the site cards.Test Plan
Related PRs and Issues
Blocked by appwrite/appwrite#10181