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

7 replace cartoon avatars with actual images #20

Merged
merged 13 commits into from
Apr 15, 2021

Conversation

machariamuguku
Copy link
Member

Fixes #7

@machariamuguku
Copy link
Member Author

@rowo This might need a design review

@rowo
Copy link

rowo commented Apr 13, 2021

@machariamuguku is there an easy way to view this in the site and a folder of the images I can access?

@machariamuguku
Copy link
Member Author

@rowo You've been given access to the images folder. I'm sharing the env file with you so you can clone and run this locally.

@peterMuriuki
Copy link
Collaborator

@rowo You've been given access to the images folder. I'm sharing the env file with you so you can clone and run this locally.

@machariamuguku please add a a screen recording or a snapshots of the page, so that they can be reviewed much quicker.

@machariamuguku
Copy link
Member Author

machariamuguku commented Apr 13, 2021

@rowo You've been given access to the images folder. I'm sharing the env file with you so you can clone and run this locally.

@machariamuguku please add a a screen recording or a snapshots of the page, so that they can be reviewed much quicker.

Before changes

Screenshot 2021-04-13 at 09 32 08

After Changes
Screenshot 2021-04-13 at 13 22 33

Copy link
Contributor

@moshthepitt moshthepitt left a comment

Choose a reason for hiding this comment

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

I could be wrong, but it looks like the new image for "Nutrition" seems to have its aspect ratio messed up. Could you fix that please, and investigate that it does not happen for the other two?

@machariamuguku
Copy link
Member Author

I could be wrong, but it looks like the new image for "Nutrition" seems to have its aspect ratio messed up. Could you fix that please, and investigate that it does not happen for the other two?

fixed

@moshthepitt
Copy link
Contributor

LGTM but let's let @rowo take a look

@rowo
Copy link

rowo commented Apr 13, 2021

I will edit the image coloring

@machariamuguku On the MIECD site, are the images always in the ratio in the screenshot, #20 (comment), or do they follow the width of the grey container behind it? I guess I’m wondering at narrow and wide browser widths, do they images look okay at their current ratio? I guess it doesn't matter if the images are all the same size and behave the same.

@machariamuguku
Copy link
Member Author

I will edit the image coloring

@machariamuguku On the MIECD site, are the images always in the ratio in the screenshot, #20 (comment), or do they follow the width of the grey container behind it? I guess I’m wondering at narrow and wide browser widths, do they images look okay at their current ratio? I guess it doesn't matter if the images are all the same size and behave the same.

They scale linearly, following the width of the container at a ratio. They always fit the container without stretching

@rowo
Copy link

rowo commented Apr 14, 2021

@machariamuguku here are some images where I matched the nutrition image to same ratio as the other two and tried to normalize the color. In my opinion, it would look better to put the images within the container now that they are full-bleed photos and not cutout illustrations, but I understand if that's not possible for the project.

Nutrition  (1)-sm
Pregnancy  (2)-sm
PNC   NBC (2)-sm
Nutrition  (1)-md
Pregnancy  (2)-md
PNC   NBC (2)-md
Nutrition  (1)-lg
Pregnancy  (2)-lg
PNC   NBC (2)-lg

@machariamuguku
Copy link
Member Author

In my opinion, it would look better to put the images within the container now that they are full-bleed photos and not cutout illustrations

@rowo I could try that.

Are you thinking more of this
card_1

or this?
card_2

@machariamuguku
Copy link
Member Author

machariamuguku commented Apr 14, 2021

@rowo In hindsight, due to timelines, I think we can schedule these changes for the future

@moshthepitt
Copy link
Contributor

I think we can schedule these changes for the future

@machariamuguku could you add an issue for this?

@peterMuriuki peterMuriuki merged commit c8fb249 into main Apr 15, 2021
@peterMuriuki peterMuriuki deleted the 7-replace-cartoon-avatars-with-actual-images branch April 15, 2021 10:57
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.

Dashboards Home page
4 participants