Skip to content
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

generate screenshots for our demos + requirements.lock.txt #131

Closed
edublancas opened this issue Feb 29, 2024 · 27 comments
Closed

generate screenshots for our demos + requirements.lock.txt #131

edublancas opened this issue Feb 29, 2024 · 27 comments
Assignees
Labels
stash Label used to categorize issues that will be worked on next

Comments

@edublancas
Copy link
Contributor

we have a new explore page where we'll showcase our examples

we need to generate one image per demo, no need to have them all at once, we can generate a batch and open a PR

also, no need to deploy them, we can run them locally and take the screenshot

since some examples might not work due to library API changes, we might need to update the code or downgrade versions, whatever is easier on each case, once we get them working, we should record the versions with pip freeze > requirements.lock.txt

@edublancas edublancas added the stash Label used to categorize issues that will be worked on next label Feb 29, 2024
@edublancas
Copy link
Contributor Author

@yafimvo @idomic we're gonna start taking screenshots of all out demos so they show up in the explore page. is there a recommended resolution for them?

@edublancas
Copy link
Contributor Author

we did a lot of moving in the examples recently. if you find anything that's weird, please open an issue (e.g., broken links from the documentation to the examples) @neelasha23

@neelasha23
Copy link
Contributor

where do we need to store the screenshots? In the same folder as each project? Or some common folder for all projects? @edublancas

@edublancas
Copy link
Contributor Author

same repository, let's also add them to the readme, below the title and short explanation:

# title

short explanation

-- add image here

explanation continues

let's wait for @idomic @yafimvo to give their feedback on the image size, you can start generating the requirements.lock.txt in the meantime

@neelasha23
Copy link
Contributor

Added a couple of samples: #134
Please check if format is correct.
@edublancas

@yafimvo
Copy link
Contributor

yafimvo commented Mar 1, 2024

@yafimvo @idomic we're gonna start taking screenshots of all out demos so they show up in the explore page. is there a recommended resolution for them?

The w/h proportion is approximately 1.9, but there is no specific recommended resolution.

Below are some images I used in development:

Archive.zip

Both of them look nice

@edublancas
Copy link
Contributor Author

thanks @yafimvo

@neelasha23 I'll give you access to the /explore page (we'll use the screenshots there as well) so you can test sizes

opengraph recommends:

Use images that are at least 1200 x 630 pixels for the best display on high resolution devices.

so let's start with that. we also need to ensure the images are small so they load fast. I think webp is a good candiate.

@neelasha23 expect a message from me on slack

@neelasha23
Copy link
Contributor

1200 x 630:

screenshot

1400 x 673:

screenshot (2)

I think the 1st one is not that good and we should go for a bit higher resolution? @edublancas

@neelasha23
Copy link
Contributor

Also, do we need to add all example to the explore page? That means we need to keep the applications deployed? Any specific account in which to deploy? @edublancas

@edublancas
Copy link
Contributor Author

re resolution: yeah, you can experiment a bit and decide which resolution and aspect ratio is best. also, you might wanna zoom in using the browser so the text is more readable

re deployments: good point. I think for now, let's link to the source code in this repository. example

I also realized that some of our apps are APIs (no UI), in that case, we can create an image with the framework's logo (flask, fastapi, etc)

@edublancas
Copy link
Contributor Author

@neelasha23 what's the status? please open small PRs, don't wait to have all screenshots

@neelasha23
Copy link
Contributor

The thumbnails were not coming up in explore page. The fix was got deployed. The images that I generate are a bit small for the thumbnail. I'll just resize the images and update the PR

@neelasha23
Copy link
Contributor

re deployments: good point. I think for now, let's link to the source code in this repository. example

I think the link can only be a valid app URL as per the docs. passing any other link is throwing error:

{"type":"https://cloud-prod.ploomber.io/docs","title":"Bad Request","detail":"Invalid project URL."}

@edublancas

@edublancas
Copy link
Contributor Author

edublancas commented Mar 6, 2024

@yafimvo @idomic can we remove this URL validation. some of our examples are not deployed anymore and we want to link to the README.md, we might decide to deploy them at some point but right now it'd be easier to just link to the readme

@neelasha23 please work on #139 (and the other assigned issues) until we unblock this

@neelasha23
Copy link
Contributor

neelasha23 commented Mar 6, 2024

Also, webp is not supported. so adding png files as of now

(env1) (base) neelashasen@192 book-recommender % curl -X POST https://cloud-prod.ploomber.io/public/projects \
     -H "api_key: Y0MrJ5Kf6gCwLqxYnFmojZpyLSfHgTsYLSUKxyOJAGk" \
     -H "Content-Type: multipart/form-data" \
     -F "cover_image=@screenshot.webp;type=image/webp" \
     -F "project={\"title\":\"Book Recommender\",\"description\":\"An app for recommending books\",\"type\":\"panel\",\"link\":\"https://twilight-snowflake-7751.ploomberapp.io\"}"
{"type":"https://cloud-prod.ploomber.io/docs","title":"Bad Request","detail":"The image 'screenshot.webp' is not a valid image.\n Allowed files are : ['jpg', 'jpeg', 'png']"}%  

@edublancas

@edublancas
Copy link
Contributor Author

ok, let's do another format then

@neelasha23
Copy link
Contributor

I also tried different sizes but there is still some padding around the image. I'm not sure if it was designed that way. maybe the padding can be reduced?

Screenshot 2024-03-06 at 11 06 47 PM

@yafimvo @edublancas

@yafimvo
Copy link
Contributor

yafimvo commented Mar 7, 2024

@yafimvo @idomic can we remove this URL validation. some of our examples are not deployed anymore and we want to link to the README.md, we might decide to deploy them at some point but right now it'd be easier to just link to the readme

@neelasha23 please work on #139 (and the other assigned issues) until we unblock this

Yes, in this case, I need to add one more parameter - project_name.
Do we still want the button text would be View App even for repo links?

I also realized that some of our apps are APIs (no UI), in that case, we can create an image with the framework's logo (flask, fastapi, etc)

As for the default images, I can create a mechanism that if there is no image we can show the framework logo.

I also tried different sizes but there is still some padding around the image. I'm not sure if it was designed that way. maybe the padding can be reduced?

yes, the padding is part of the design. why do we want to reduce it?

As part of this PR I'll also add webp support.

If you think I should include anything else please let me know.

@edublancas @neelasha23

@neelasha23
Copy link
Contributor

yes, the padding is part of the design. why do we want to reduce it?

The text would look a bit clearer and bigger if we reduce a bit of the padding. wdyt @edublancas

@yafimvo
Copy link
Contributor

yafimvo commented Mar 7, 2024

yes, the padding is part of the design. why do we want to reduce it?

The text would look a bit clearer and bigger if we reduce a bit of the padding. wdyt @edublancas

@neelasha23 It won't make the difference

image

The issue is that the image has a lot of details in it, and that's why the text will always look "small". we can show a smaller part of the app, for example

image

this is the image

test-1

@edublancas
Copy link
Contributor Author

Yes, in this case, I need to add one more parameter - project_name.

why is a new parameter needed?

Do we still want the button text would be View App even for repo links?

As for the default images, I can create a mechanism that if there is no image we can show the framework logo.

sounds good

yes, the padding is part of the design. why do we want to reduce it?

I think the padding is fine

As part of this PR I'll also add webp support.

sounds good

@neelasha23
Copy link
Contributor

  1. Do we need to add these projects?
    docker/jupyter-kernel-gateway, docker/jupyter-kernel-gateway-gpu: These have only Dockerfile and requirements.txt files. jupyterlab and jupyterlab-gpu: These two have Dockerfile, requirements.txt and config files.

  2. fastapi/ai-helper (Customer chatbot app): I'm not sure if this needs to be added since we are building a simpler version without Haystack?

  3. Pending apps: - Solara Video RAG

@edublancas

@edublancas
Copy link
Contributor Author

  1. that's ok, you can ignore those
  2. ignore that
  3. ignore that as well

we can come back to those later if needed.

are those the only ones left?

@neelasha23
Copy link
Contributor

yes i have added all the rest @edublancas

@idomic
Copy link
Contributor

idomic commented Mar 18, 2024

Did you add them to the zip files too?
And are they used anywhere?

When I did some check on Friday I didn't see either

@neelasha23
Copy link
Contributor

No I didn't add the lock files to the zip as it was not in the scope of the issue. Please refer to this comment @idomic

@edublancas
Copy link
Contributor Author

edublancas commented Mar 18, 2024

we're not adding .zip for all examples, we only add them for the ones used in the frontend

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stash Label used to categorize issues that will be worked on next
Projects
None yet
Development

No branches or pull requests

4 participants