Add action to push screenshots to conda store

kcpevey commented 8 months ago

Fixes https://github.com/conda-incubator/conda-store/issues/570

Description

This workflow collects screenshots which have stored as artifacts in test.yml and opens a PR on conda-store to update the docs.

This pull request:

adds gh action to push screenshots to conda-store

Pull request checklist

[x] Did you test this change locally?
[ ] Did you update the documentation (if required)?
[ ] Did you add/update relevant tests for this change (if required)?

Additional information

We originally discussed triggering this from a comment on a PR. I discovered that the only Action trigger, issue_comment, would trigger this to run for every comment on every issue. I feel like that is a lot of wasted compute even if the first thing it did was check to see if its a PR and shut it down. For this reason, I've decided to trigger on label instead.

# There are two ways to trigger this workflow:
# 1. Trigger by adding the `update_screenshots` label. 
#    The workflow will run immediately after you add the label. It is
#    also expecting artifacts which have been generated in test.yml. 
#    Therefore, this cannot be triggered until test.yml is complete. 
# 2. Trigger by workflow dispatch.
#    Specify the PR number with the artifacts you'd like to use.
#    Again, the test.yml workflow must be complete.
#
# If a PR has already been opened on the conda-store repo from this
# action, a rerun of this action will attempt to update the PR, not
# create a new one.

I looked at triggering on the completion of test.yml, but that would trigger the workflow off of main and we'd be forced to use the "latest" generated artifacts, rather than PR-specific.

This will require a PAT to open up the PR on the conda-store side. It is currently looking for secrets.PAT.

kcpevey commented 8 months ago

I've tested this approach on dummy repos but this will still take me some tuning to get everything right in place.

TODO:

get secrets sorted out
add more screenshots to playwright to cover what's already in conda-store
- make sure the words of the doc aren't specific to the old screenshot (i.e. has the doc been updated to the new UI)
sort out where these need to land (in the new docs folder or overwrite the old screenshots?)

kcpevey commented 8 months ago

@pavithraes it looks like a lot of the images in /docusaurus-docs/conda-store-ui/images have red boxes around the area of interest. We will lose those if we let playwright generate the images. Is that ok, or should we avoid updating those images with playwright?

kcpevey commented 7 months ago

Both the workflow dispatch trigger and the issue comment trigger will only work once the workflow is committed to main. I'm leaving the label trigger in for now so that I have a way to call it.

pavithraes commented 7 months ago

@kcpevey Thanks for working on this!

it looks like a lot of the images in /docusaurus-docs/conda-store-ui/images have red boxes around the area of interest. We will lose those if we let playwright generate the images. Is that ok, or should we avoid updating those images with playwright?

Yes, it's OK to override the images with red highlights. I think keeping the screenshots up-to-date has a greater impact, and we can always make things clearer in the text if needed. :)

kcpevey commented 7 months ago

Adding the mouse indicator and hover is not available straight out the box with playwright. So this image, for example:

Will not be possible. This may be possible with an extra extension such as mouse-helper (mentioned here) but we'd need to work out if that's possible in python playwright.

Another option would be to run playwright in the Trace Viewer which has snapshots, one of which would include a red circle where the mouse click happened. This would take snapshots before, during, and after each interaction with the page. So it would give us some helpful functionality, but it feels like overkill.

For this work, I'm going to leave off any mouse visualization or click indicators.

kcpevey commented 7 months ago

There is a duplicate image in the current docs. I'm going to continue on with version-select.png and recommend that versions-view.png be removed.

kcpevey commented 7 months ago

The pip section of the yaml editor now populates by default. So the two images yaml-editor.png and pip-section.png are now going to be the same image unless I explicitly remove the pip section for yaml-editor.png. I'm not sure what the best choice would be here. Maybe depends on what the docs using these files look like (I haven't looked them up yet)?

ETA: actually, it only populates by default if you are editing an existing env, not if you are creating a new env. I've updated the images so that I think they appropriately match the existing images.

github-actions[bot] commented 7 months ago

A PR to conda-store has been opened with updated screenshots - https://github.com/conda-incubator/conda-store/pull/730

kcpevey commented 7 months ago

I recommend leaving the label trigger for pushing the images in place and removing in a subsequent PR so that we can be sure that the other triggers work once they are in main.

github-actions[bot] commented 7 months ago

A PR to conda-store has been opened with updated screenshots - https://github.com/conda-incubator/conda-store/pull/731

kcpevey commented 7 months ago

Finally, this is ready for review!

If you check the PR on the docs side, you can see the comparison of the new images. Because we no longer have the "red boxes" to direct your attention to a portion of the screen, I decided to crop the images to only include the region of interest. If you check the netlify docs, the image sizes are now inappropriate. I recommend that this can be merged and that the auto generated PR on the other side be worked on to resize the images in the docs.

trallard commented 7 months ago

If you check the https://github.com/conda-incubator/conda-store/pull/731, you can see the comparison of the new images. Because we no longer have the "red boxes" to direct your attention to a portion of the screen, I decided to crop the images to only include the region of interest.

I actually prefer this approach -> only show the relevant UI sections vs the whole UI so this is a +1 from me

@kcpevey this is ready to merge on my end. I left a couple of non-blocking suggestions and comments, so feel free to address them and then merge them accordingly. Thank you for this PR we will finally keep all our screenshots up-to-date 🎉

kcpevey commented 6 months ago

This is ready to merge (failing CI is outside of this PR - being resolved here https://github.com/conda-incubator/conda-store-ui/pull/360)

kcpevey commented 3 months ago

Hmm. I followed the instructions for local deployment with and without docker and all my tests pass locally but based on the video, the CI is bringing in a different version of the UI than what I see locally. I assume that CI is correct and something is old/cached with my local version, but I'm not sure what it is. I've merged main back into my branch so theoretically, my branch is up to date. What else could be wrong? @gabalafou ?

@gabalafou gave me the info I needed - the new version of the UI is provided on port 8000 and an older version of the UI is on 8080.

conda-incubator / conda-store-ui