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

NTP Fall 2019 wallpapers #6904

Closed
karenkliu opened this issue Nov 13, 2019 · 28 comments · Fixed by brave/brave-core#4264 or brave/brave-core#4398
Closed

NTP Fall 2019 wallpapers #6904

karenkliu opened this issue Nov 13, 2019 · 28 comments · Fixed by brave/brave-core#4264 or brave/brave-core#4398

Comments

@karenkliu
Copy link

karenkliu commented Nov 13, 2019

Description

Swap out all of our current wallpapers on the new tab with a fresh set of images. Purpose:

  • prevent the images from feeling stale
  • give other photographers/creators a chance to showcase their work
  • it's fun

Designs

wallpaper preview

Assets

compressed JPG and WEBP
image assets.zip

photo creds + links
attribution.pdf

Test plan

Images should match those shown in [redacted]
(ones in red should not be showing)

@karenkliu
Copy link
Author

Maybe apply this .webp compression method in brave/brave-core#3735 as well

@jhreis
Copy link

jhreis commented Nov 19, 2019

I put all the data together in a JSON file. @karenkliu if you could review this data, just for a second pair of eyes that would be great.

Also, do we have a website for "Louis Kim"?

ntp-data.json.zip

center is required for mobile to center the image correctly on mobile.

@karenkliu
Copy link
Author

karenkliu commented Nov 19, 2019

@jhreis We're still waiting on a link from Louis Kim; he's been traveling and says he can get it to us "tonight". So I'll post that link here tomorrow.

I checked out the JSON file and noticed a couple things:

  • They all have center set at '0' except for xavier-balderas-cejudo.jpg, which set at 2560; is that a mistake?
  • I'd like to help determine what should be the portrait center for these images, how do I do that?
  • oliwier gesla.jpg is the only file name with a space in it; is that going to be a problem? My bad for overlooking that when I was exporting stuff.

Other than those things, the file name, credits, and links all seem fine.

@jamesmudgett
Copy link

jamesmudgett commented Nov 19, 2019

// center points for mobile portrait:

A - 1280
B - 1630
C - 1277
D - 1975
E - 815
F - 1160
G - 988
H - 682
I - 0
J - 480
K - 1200
L - 780
M - N/A
N - 2185
O  - 1205
P - 1330

@jhreis
Copy link

jhreis commented Nov 20, 2019

I updated the above JSON file with the center points. Thanks @jamesmudgett and @karenkliu

The space in the file name does not impact iOS, if it is problematic for other platforms I can update JSON file.

I think the JSON file is only needed for mobile probably, and only is used in portrait mode.

@rebron rebron added the priority/P3 The next thing for us to work on. It'll ride the trains. label Nov 20, 2019
@karenkliu
Copy link
Author

@jhreis The attribution link for Louis Kim is louiskimphotography.com

@deeppandya
Copy link

@karenkliu link louiskimphotography.com doesn't work. can you check with the artist ?

@karenkliu
Copy link
Author

Sure, I sent them a message. Will post when they get back to me.

@karenkliu
Copy link
Author

@deeppandya Please go ahead and use that same link. Louis said:

There was a problem with domain mapping. I checked again with my hosting service and they did a manual intervention. Should be up in 24-48 hours.

@fmarier
Copy link
Member

fmarier commented Nov 26, 2019

@karenkliu Do you still have a record of the URLs where you downloaded the images?

I'm asking because, in addition to the photographer's attribution link, we now also keep track of the image URL and the license: https://github.com/brave/brave-core/blob/master/components/brave_new_tab_ui/data/LICENSE

(This is now visible in brave://credits under "Background images".)

@karenkliu
Copy link
Author

@fmarier Sure, here you go:
2019-11 photo attribution and license.pdf

@fmarier
Copy link
Member

fmarier commented Nov 27, 2019

@karenkliu
Copy link
Author

@fmarier Ah okay, got it - I'll do that for next time. This is what I think is right:

1: https://unsplash.com/photos/uwbajDCODj4
2: https://unsplash.com/photos/mawU2PoJWfU
3: https://unsplash.com/photos/4knR_YzeUVc
4: https://unsplash.com/photos/C9jP5AlgcuQ
5: https://unsplash.com/photos/sfgH9dXcMRw
6: Contributor sent the hi-res version through email
7: Contributor sent the hi-res version through email
8: https://unsplash.com/photos/4xv3lqnanYc
9: Contributor sent the hi-res version through email
10: https://unsplash.com/photos/veMnvjmfoxw
11: Contributor sent the hi-res version through email
12: Contributor sent the hi-res version through email
13: Contributor sent the hi-res version through email
14: Contributor sent the hi-res version through email
15: https://unsplash.com/photos/kxIE049IZ1g

As you can see, the URLs in the license file are only available for Unsplash images. The images we selected from direct contributor submissions were not acquired through a link - the contributor supplied us with a high-resolution image to use directly through email. A lot of times the images shown on a photographer's site are not high enough resolution for us to use as a wallpaper because they don't want unauthorized use of their images, so we reached out to them directly to acquire those images. I don't think a URL in the license file would be applicable for those direct contributor submissions.

@fmarier
Copy link
Member

fmarier commented Nov 27, 2019

A lot of times the images shown on a photographer's site are not high enough resolution for us to use as a wallpaper because they don't want unauthorized use of their images, so we reached out to them directly to acquire those images. I don't think a URL in the license file would be applicable for those direct contributor submissions.

Ah good point. I didn't realize they were emailing them to us directly.

I wonder what would be most useful to show in the LICENSE file then. Maybe the low-res image from the photographer's website/Instagram so that people can see where we sourced the images (understanding it may not the exact version that we put in Brave)? The nice thing about having a URL is that you can discover other photos from that photographer or comment on it, but I guess people already do that with the attribution link after finding the exact photo.

Anyways, the point of this is not to create more work for ourselves, just to keep track of where things are from. What would you think about this (for next time):

  1. If we find a photo (low-res or high-res) on a website / Instagram, then we can link to it from the LICENSE file -- we basically just note the URL where we originally found it.
  2. If we get emailed a photo directly (i.e. we didn't find it, it came to us), then we can list it as "sent by contributor through email".

@rebron rebron assigned cezaraugusto and unassigned rebron Dec 3, 2019
@karenkliu
Copy link
Author

@fmarier That sounds good to me! For this round I think "sent by contributor through email" would suffice

@rebron rebron added closed/stale Issue is no longer relevant, perhaps because the feature it refers to has been deprecated. feature/new-tab priority/P3 The next thing for us to work on. It'll ride the trains. and removed feature/new-tab priority/P3 The next thing for us to work on. It'll ride the trains. closed/stale Issue is no longer relevant, perhaps because the feature it refers to has been deprecated. labels Dec 17, 2019
@cezaraugusto
Copy link
Contributor

@fmarier do you think we should update some file/license file after this work lands? I should have a PR open soon with this update.

@LaurenWags
Copy link
Member

Marking as QA/Blocked for now since brave/brave-core#4398 was reverted.

@bsclifton
Copy link
Member

bsclifton commented Mar 19, 2020

@LaurenWags it should be there actually- I did open a revert, but we fixed a different way 😄 Let me make a note there (my bad)

edit:
Comment left brave/brave-core#4398 (comment) 😄 Removing QA/Blocked label

@kjozwiak
Copy link
Member

kjozwiak commented Mar 20, 2020

@bsclifton @rebron @cezaraugusto @karenkliu just double checking, these are the expected images correct?

@rebron
Copy link
Collaborator

rebron commented Mar 20, 2020

* https://raw.githubusercontent.com/brave/brave-core/8c4bcfee376033fe8d5528f00657b3b603c16d8c/components/img/newtab/backgrounds/anders-jilden.webp

@kjozwiak
Copy link
Member

Thanks @rebron 👍Missed it when copying/pasting the list I pasted into the uplift channel on Slack.

@kjozwiak
Copy link
Member

kjozwiak commented Mar 27, 2020

Verification PASSED on macOS 10.15.3 x64 Catalina using the following build:

Brave | 1.7.76 Chromium: 80.0.3987.149 (Official Build) dev (64-bit)
-- | --
Revision | 5f4eb224680e5d7dca88504586e9fd951840cac6-refs/branch-heads/3987_137@{#16}
OS | macOS Version 10.15.3 (Build 19D76)

Verification passed on

Brave 1.7.78 Chromium: 80.0.3987.149 (Official Build) dev (64-bit)
Revision 5f4eb224680e5d7dca88504586e9fd951840cac6-refs/branch-heads/3987_137@{#16}
OS Windows 10 OS Version 1803 (Build 17134.1006)

Verification passed on

Brave 1.7.78 Chromium: 80.0.3987.149 (Official Build) dev (64-bit)
Revision 5f4eb224680e5d7dca88504586e9fd951840cac6-refs/branch-heads/3987_137@{#16}
OS Ubuntu 18.04 LTS

@kjozwiak
Copy link
Member

@rebron @SergeyZhukovsky @aekeus should these make their way into Android as well? If so, can you please add the OS/Android tag for this issue?

@rebron
Copy link
Collaborator

rebron commented Mar 27, 2020

NTP wallpapers are already updated on Android, so no additional work for android here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment