Page MenuHomePhabricator

Replace custom_types_figure_one image to highlight SimpleDataFrame
Changes PlannedPublic

Authored by yichendai on Fri, Oct 2, 5:17 PM.

Details

Reviewers
max
Summary

asdf

Test Plan

test

Diff Detail

Repository
R1 dagster
Branch
rev8
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

max requested changes to this revision.Fri, Oct 2, 5:43 PM
max added inline comments.
docs/next/public/assets/images/tutorial/custom_types_figure_one_simpleDataFrame.png
1 ↗(On Diff #23155)

please replace existing image or delete it

This revision now requires changes to proceed.Fri, Oct 2, 5:43 PM

Update impage - Delete old image custom_types_figure_one

Updating D4641: Replace custom_types_figure_one image to highlight SimpleDataFrame

Modifed custom_types_figure_one image to fix dead link

this is fine as a standalone change, i am requesting changes just so we can discuss some of the broader issues. if we are going to use this green circle highlight idiom, we should use it elsewhere in the tutorial as well - it doesn't really make sense for this to be the only screenshot with salient elements highlighted. i also think we need a plan for how we're going to recreate these screenshots when the UI changes. the previous screenshots were generated automatically using a script: https://github.com/dagster-io/dagster/blob/master/docs/generate_screenshots.test.js -- if we aren't going to update this script to generate the highlights as well, i think we should consider writing instructions for whatever manual work is required to highlight the screenshots.

max requested changes to this revision.Wed, Oct 7, 2:40 PM
This revision now requires changes to proceed.Wed, Oct 7, 2:40 PM
In D4641#126638, @max wrote:

if we are going to use this green circle highlight idiom, we should use it elsewhere in the tutorial as well - it doesn't really make sense for this to be the only screenshot with salient elements highlighted. i also think we need a plan for how we're going to recreate these screenshots when the UI changes. the previous screenshots were generated automatically using a script: https://github.com/dagster-io/dagster/blob/master/docs/generate_screenshots.test.js -- if we aren't going to update this script to generate the highlights as well, i think we should consider writing instructions for whatever manual work is required to highlight the screenshots.

I totally agree with you on making all the UI uniform, but I'm not sure about generating them automatically by the script. We only need to make highlights for those concepts being introduced explicitly in the paragraph. The highlights will help the readers quickly become familiar with Dagit and some new concepts. I can read and note how much manual work are there.

This comment was removed by yichendai.