portico: Add script to auto-generate message screenshots /for/research. #30061

roanster007 · 2024-05-11T11:48:17Z

Previously, we manually generated screenshots on /for/research and other landing pages, which makes them difficult to maintain.

This PR adds a script - generate-user-messages-screenshot, which expects the path of the filename containing the thread conversation and path where screenshot needs to be generated, and generates screenshot at the mentioned location.

Fixes #30016

Testing: ./tools/screenshots/generate-user-messages-screenshot --thread tools/screenshots/interactive_messaging.json

Screenshots and screen captures:

Self-review checklist

Self-reviewed the changes for clarity and maintainability
(variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

Explains differences from previous plans (e.g., issue description).
Highlights technical choices and bugs encountered.
Calls out remaining decisions and concerns.
Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

Each commit is a coherent idea.
Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

Visual appearance of the changes.
Responsiveness and internationalization.
Strings and tooltips.
End-to-end functionality of buttons, interactions and flows.
Corner cases, error conditions, and easily imagined bugs.

zulipbot · 2024-05-11T11:48:24Z

Hello @zulip/server-misc members, this pull request was labeled with the "area: portico" label, so you may want to check it out!

tools/generate-user-messages-screenshot

timabbott · 2024-05-13T00:31:14Z

Overall, this looks great; posted a few comments but I'm generally in favor of merging this sort of thing and then doing iterative refinements on top of it.

roanster007 · 2024-05-13T10:43:36Z

Better would be to use try/finally to ensure they get cleaned up even if the script crashes.

I updated to clean up the messages added in the finally block.

Oh, I see, it's the next commit; yeah let's at least do the directory structure change proposed in my comment to give this cluster of tools its own directory to avoid cluttering the tools/ root with at least one file per screenshot

I moved generate-integration-docs-screenshots and message-screenshot to tools/screenshots directory as prep commit.
Added the new script files there as well.

Should the data for the message conversation (interactive_messaging.json) also be moved to this dir? Currently I added it in static/images/landing-page/research/interactive_messaging.json since that is where the current images for for/research page reside.

alya · 2024-05-13T23:01:43Z

static/images/landing-page/research/interactive_messaging.json

+            },
+            {
+                "sender": "Zoe Davis",
+                "content": "@**everyone** Video call is happening now! [Click to join video call](#)"


Let's change [Click to join video call] -> [Join video call.], since we changed the default text the button inserts.

alya · 2024-05-13T23:06:02Z

Let's make the times more realistic, so that it doesn't look like all the messages were sent at the same time. I think the times in the screenshots that are currently on the website are fine, but I'm not super picky.

alya · 2024-05-13T23:07:35Z

"Today" is generally good for the date, and thanks for updating the global time. But let's make the storyline be that the call gets scheduled the day before, and then the "happening now" message is sent at the time that the call was scheduled for.

alya · 2024-05-13T23:08:47Z

@terpimost Could you please share images we should use for avatars for these conversations?

static/images/landing-page/research/interactive_messaging.json

timabbott · 2024-05-13T23:59:05Z

Should the data for the message conversation (interactive_messaging.json) also be moved to this dir? Currently I added it in static/images/landing-page/research/interactive_messaging.json since that is where the current images for for/research page reside.

Yeah, let's have it live in this directory; it's good to have all the source materials in one place.

tools/screenshots/generate-integration-docs-screenshot

tools/screenshots/generate-user-messages-screenshot

timabbott · 2024-05-14T00:03:33Z

We can probably not block on finalizing the content of the json file and what avatars to use for merging the tool, so I'm good with merging this PR as soon as my comments above are resolved, and the rest will probably feel nice to do as follow-ups so I don't need to keep reading the core system code, but of course it'd be great to check off as many things as we can from Alya's feedback.

This commit relocates all the scripts in the tools directory which are used for auto-generating screenshots to the new tools/screenshots directory to avoid cluttering the tools/ root.

This commit adds the "forged_timestamp" parameter to the "internal_prep_stream_message" method of "actions/message_send". This is a preparatory commit, that can be used for sending messages at a forged time in scripts for generating screenshots of messages.

Previously, we manually generated screenshots on /for/research and other landing pages, which makes them difficult to maintain. This PR adds a script - generate-user-messages-screenshot, which expects the path of the filename containing the thread conversation and path where screenshot needs to be generated, and generates screenshot at the mentioned location. Fixes zulip#30016

This commit utilizes the script added for auto-generating screenshots of message threads to generate screenshots for the "/for/recovery" page. This commit adds the message data for generating the screenshots.

roanster007 · 2024-05-14T17:48:51Z

@alya addressed reviews:

It doesn't matter (since it's not visible in the screenshot), but it might be a bit nicer to use character names from https://zulip.com/ than Abraham Lincoln, etc.?

Let's change [Click to join video call] -> [Join video call.], since we changed the default text the button inserts.

Let's make the times more realistic, so that it doesn't look like all the messages were sent at the same time. I think the times in the screenshots that are currently on the website are fine, but I'm not super picky.

"Today" is generally good for the date, and thanks for updating the global time. But let's make the storyline be that the call gets scheduled the day before, and then the "happening now" message is sent at the time that the call was scheduled for.

And updated screenshots accordingly.

roanster007 · 2024-05-14T17:53:21Z

@timabbott

I Added a preparatory commit so as to be able to get SendMessageRequest with forged_timestamp using internal_prep_stream_message

Addressed reviews:

Yeah, let's have it live in this directory; it's good to have all the source materials in one place.

changed the location of the json file to `tools/screenshots.

No, I mean if access_message fails. I guess it shouldn't as long as the current user isn't a guest; it just seems weird to query this way instead of Message.objects.filter(id__in=thread_message_ids)

Removed access_message and updated with this.

Never rename something without doing a git grep to see if it's referenced elsewhere:

Updated tools/generate-integration-docs-screenshots to tools/screenshots/generate-integration-docs-screenshots in docs.

alya · 2024-05-14T19:36:29Z

Works for me!

timabbott · 2024-05-14T23:17:49Z

Merged, thanks @roanster007!

timabbott · 2024-05-14T23:22:45Z

A few follow-up requests:

Can we disable the log output for sending outgoing emails in the tool, and give it proper usage output, to make it a bit more pleasant to use?
Can you document this on localhost:9991/devtools?
Probably you can start working through writing the JSON files for other screenshots on /for/* landing pages.

alya · 2024-05-16T00:10:11Z

Probably you can start working through writing the JSON files for other screenshots on /for/* landing pages.

Please skip /for/business for now; I need to do some content work there.

roanster007 · 2024-05-17T17:53:39Z

I'll work over next few days.

zulipbot added size: XL area: portico priority: high labels May 11, 2024

roanster007 commented May 11, 2024

View reviewed changes

tools/generate-user-messages-screenshot Outdated Show resolved Hide resolved

roanster007 force-pushed the iss-30016 branch from fb28afd to 4a3012d Compare May 11, 2024 16:32

timabbott reviewed May 13, 2024

View reviewed changes

tools/generate-user-messages-screenshot Outdated Show resolved Hide resolved

roanster007 force-pushed the iss-30016 branch from 4a3012d to 63c2a20 Compare May 13, 2024 10:36

alya reviewed May 13, 2024

View reviewed changes

static/images/landing-page/research/interactive_messaging.json Outdated Show resolved Hide resolved

timabbott reviewed May 14, 2024

View reviewed changes

tools/screenshots/generate-integration-docs-screenshot Show resolved Hide resolved

timabbott reviewed May 14, 2024

View reviewed changes

tools/screenshots/generate-user-messages-screenshot Outdated Show resolved Hide resolved

roanster007 added 4 commits May 14, 2024 21:32

refactor: Relocate screenshots scripts to new tools/screenshots dir.

d4e0626

This commit relocates all the scripts in the tools directory which are used for auto-generating screenshots to the new tools/screenshots directory to avoid cluttering the tools/ root.

portico: Generate screenshots for the /for/research page.

ff15ff5

This commit utilizes the script added for auto-generating screenshots of message threads to generate screenshots for the "/for/recovery" page. This commit adds the message data for generating the screenshots.

roanster007 force-pushed the iss-30016 branch from 63c2a20 to ff15ff5 Compare May 14, 2024 17:21

alya added the integration review Added by maintainers when a PR may be ready for integration. label May 14, 2024

timabbott merged commit 399dfe1 into zulip:main May 14, 2024
13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

portico: Add script to auto-generate message screenshots /for/research. #30061

portico: Add script to auto-generate message screenshots /for/research. #30061

roanster007 commented May 11, 2024 •

edited

zulipbot commented May 11, 2024

timabbott commented May 13, 2024

roanster007 commented May 13, 2024

alya May 13, 2024

roanster007 May 14, 2024

alya commented May 13, 2024 •

edited

alya commented May 13, 2024 •

edited

alya commented May 13, 2024

timabbott commented May 13, 2024

timabbott commented May 14, 2024 •

edited

roanster007 commented May 14, 2024

roanster007 commented May 14, 2024 •

edited

alya commented May 14, 2024

timabbott commented May 14, 2024

timabbott commented May 14, 2024

alya commented May 16, 2024

roanster007 commented May 17, 2024 •

edited

portico: Add script to auto-generate message screenshots /for/research. #30061

portico: Add script to auto-generate message screenshots /for/research. #30061

Conversation

roanster007 commented May 11, 2024 • edited

zulipbot commented May 11, 2024

timabbott commented May 13, 2024

roanster007 commented May 13, 2024

alya May 13, 2024

Choose a reason for hiding this comment

roanster007 May 14, 2024

Choose a reason for hiding this comment

alya commented May 13, 2024 • edited

alya commented May 13, 2024 • edited

alya commented May 13, 2024

timabbott commented May 13, 2024

timabbott commented May 14, 2024 • edited

roanster007 commented May 14, 2024

roanster007 commented May 14, 2024 • edited

alya commented May 14, 2024

timabbott commented May 14, 2024

timabbott commented May 14, 2024

alya commented May 16, 2024

roanster007 commented May 17, 2024 • edited

roanster007 commented May 11, 2024 •

edited

alya commented May 13, 2024 •

edited

alya commented May 13, 2024 •

edited

timabbott commented May 14, 2024 •

edited

roanster007 commented May 14, 2024 •

edited

roanster007 commented May 17, 2024 •

edited