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

chore: update targets page images #1221

Merged
merged 12 commits into from
Dec 13, 2023
Merged

Conversation

Benmuiruri
Copy link
Contributor

Description

Update images in Targets page to showcase new material design cards

Old

Screenshot 2023-10-30 at 15 52 06

New

Screenshot 2023-10-30 at 15 52 29

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

@Benmuiruri Benmuiruri self-assigned this Oct 30, 2023
Copy link
Member

@andrablaj andrablaj left a comment

Choose a reason for hiding this comment

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

Thanks @Benmuiruri! Are the old screenshots used somewhere? If not, I recommend deleting them.

As I don't have the context of the target changes, I will also add @michaelkohn as a reviewer to confirm the screenshots are what we expect them to be on this page.

@Benmuiruri
Copy link
Contributor Author

@andrablaj I don't think they are being referenced anywhere else. I will delete them. Thanks for adding Michael

@michaelkohn
Copy link
Member

@andrablaj andrablaj requested review from n-orlowski and removed request for andrablaj November 1, 2023 08:17
@andrablaj
Copy link
Member

@Benmuiruri I removed myself as a reviewer and added @n-orlowski instead, as she has more knowledge about the design of the targets. It looks good to me, but I can't approve as I don't know the details of the targets change.

@Benmuiruri
Copy link
Contributor Author

Thanks @andrablaj I was not sure who to ask the review from 😄

@n-orlowski
Copy link
Contributor

@Benmuiruri these looks great! Could we keep the same aspect ratio for the images so they're consistent with the other pages? (Mobile 640x1100 / Desktop 1440x1024)

@Benmuiruri
Copy link
Contributor Author

Benmuiruri commented Nov 7, 2023

Hi @n-orlowski I have updated the images with the requested resolutions. I opened the screenshots using Preview and adjusted the size. However, I must say the targets-desktop image looks "off". Kindly confirm whether thats how it should look. Thank you.

Also, the targets-mobile does not have the bottom navigation mock up like the existing image, any suggestions a tool that can allow me to do that ?

@Benmuiruri
Copy link
Contributor Author

Hi @michaelkohn.

This is ready for a review.

@michaelkohn
Copy link
Member

michaelkohn commented Nov 13, 2023

Thanks @Benmuiruri ! For some reason it looks like some of the images are squished (as you mentioned), not a lot, but noticeable enough. Can you change the size of the actual screen you are taking a screenshot of (rather than resizing the screenshot).

Mobile looks fine but desktop looks squished horizontally

Screenshot 2023-11-13 at 10 31 07 AM

Count widgets look fine but % widgets look squished vertically

Screenshot 2023-11-13 at 10 36 51 AM

@michaelkohn
Copy link
Member

(though to be clear, i wouldn't want this to hold up the release of 4.5.0)

@Benmuiruri
Copy link
Contributor Author

Hi @michaelkohn , I updated the three images.

@latin-panda latin-panda changed the title Chore: update targets page images chore: update targets page images Dec 12, 2023
Copy link
Member

@michaelkohn michaelkohn left a comment

Choose a reason for hiding this comment

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

Looks good to me! SHIPPPPPIIITTT

@Benmuiruri Benmuiruri merged commit 9d125f6 into main Dec 13, 2023
2 checks passed
@Benmuiruri Benmuiruri deleted the chore-update-targets-images branch December 13, 2023 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants