Skip to content

Replace broken images in sponsor list #1846

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

Merged
merged 7 commits into from
Feb 23, 2018
Merged

Conversation

Lutece
Copy link
Contributor

@Lutece Lutece commented Feb 21, 2018

#1845 please look this issue

example

Resolves #1845

@jsf-clabot
Copy link

jsf-clabot commented Feb 21, 2018

CLA assistant check
All committers have signed the CLA.

@montogeek
Copy link
Member

Could you please share how it would look with the default webpack image?

@Lutece
Copy link
Contributor Author

Lutece commented Feb 21, 2018

@montogeek
I didn't understand the default webpack image.
Is it logo which have text?

@montogeek
Copy link
Member

Sorry, I mean the image you added in your changes.
This one https://github.com/webpack/webpack.js.org/blob/master/src/assets/icon-square-small-slack.png.

That screenshot is before of after your changes?

@Lutece
Copy link
Contributor Author

Lutece commented Feb 21, 2018

Before:
not_changed

After:
changed

@montogeek
Copy link
Member

montogeek commented Feb 21, 2018

That's great! It would look nicer showing the logo when the backer doesn't have an avatar:

{ supporter.avatar ? <img

@Lutece
Copy link
Contributor Author

Lutece commented Feb 21, 2018

@montogeek
Thanks :)

How long does it take to be reflected on website? :)

@montogeek
Copy link
Member

This has to be reviewed, approved and merged. No idea if merging to master automatically deploys a new version.

@Lutece
Copy link
Contributor Author

Lutece commented Feb 21, 2018

@montogeek
Is it better that change it to look like a logo even when there is no avatar too?

@montogeek
Copy link
Member

That would be cool, so it would look like a wall of avatars instead of images and texts. Can you try it to see how it looks?

@Lutece
Copy link
Contributor Author

Lutece commented Feb 21, 2018

Give me a minute :)

@Lutece
Copy link
Contributor Author

Lutece commented Feb 21, 2018

@montogeek

All_Changed:
all_changed

@montogeek
Copy link
Member

It looks really nice! Thanks!

@Lutece
Copy link
Contributor Author

Lutece commented Feb 21, 2018

@montogeek

I pushed again :)
have a nice day 👍

@skipjack skipjack added the UI/UX label Feb 23, 2018
@skipjack
Copy link
Collaborator

@Lutece I like the idea here, nice work. Doing a thorough review now and hopefully merging.

No idea if merging to master automatically deploys a new version.

@montogeek it does automatically deploy although sometimes it takes a little while.

Copy link
Collaborator

@skipjack skipjack left a comment

Choose a reason for hiding this comment

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

This looks great! Made a few minor tweaks for consistency and I'm going to quickly test locally but, assuming that all goes well, we'll get this merged soon.

@skipjack
Copy link
Collaborator

Just tested, works fine. As soon as the build passes I'll get this merged.

@Lutece
Copy link
Contributor Author

Lutece commented Feb 23, 2018

@skipjack Thanks! 👍

@skipjack skipjack changed the title To replace broken images in sponsor list Replace broken images in sponsor list Feb 23, 2018
@skipjack skipjack merged commit 3d096cb into webpack:master Feb 23, 2018
@Lutece Lutece deleted the patch-1 branch February 23, 2018 16:02
@Lutece Lutece restored the patch-1 branch February 23, 2018 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants