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

Refs #35380 -- Updated outdated images in docs/ref/contrib/admin/_images/. #18185

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nessita
Copy link
Contributor

@nessita nessita commented May 21, 2024

Trac ticket number

Refs ticket-35380

Branch description

Updated admin screenshots used in the admin docs prior to feature freeze and stable/5.1.x branch cut happening on May 22nd.

Checklist

  • This PR targets the main branch.
  • The commit message is written in past tense, mentions the ticket number, and ends with a period.
  • I have added or updated relevant docs, including release notes if applicable.

@nessita
Copy link
Contributor Author

nessita commented May 21, 2024

There are two screenshots that were updated:

@nessita nessita requested a review from a team May 21, 2024 19:33
@nessita nessita force-pushed the update-admin-docs-screeshots branch from 338de1c to 8bbe18b Compare May 21, 2024 19:34
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to switch from 800-wide to 1020-wide? (same question for the second image)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, regenerating for 800 wide.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is adding the "Django administration header" a wanted change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered it to be nicer because since both screenshots are in the same section, they felt more consistent. I think either both have the header, or none has. Any opinion?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the header of the previous image is nice for the context of the green banner (maybe) but I am ok to remove it.
It's quite nice to keep the screenshots limited so they don't need frequent updates (so perhaps best not to have the header here).

@sarahboyce
Copy link
Contributor

As a todo, we probably should update the images in docs/intro/_images/ as well

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the header of the previous image is nice for the context of the green banner (maybe) but I am ok to remove it.
It's quite nice to keep the screenshots limited so they don't need frequent updates (so perhaps best not to have the header here).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're missing selecting one of the rows here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants