mnlcpc / Unsplash-It-Sketch

A plugin to quickly include great looking image from Unsplash.com in your Sketch projects.
421 stars 23 forks source link

Use new API and search terms #3

Closed nolastan closed 8 years ago

nolastan commented 8 years ago

This should make it real easy for folks to make realistic mocks. e.g. imagine a restaurant website where some images should be of food, some of people, and some of a storefront. Just name the layers accordingly and you instantly have beautiful photos.

image

fhuel commented 8 years ago

Thanks for the pull request, this is a really good idea. Using the source API, instead of the Unsplash.it one, would remove the possibility to request the images passing options ( b/w, blur, gravity ). I have no data about how much this functionality is used... I don't use it very much personally, but I think it can be useful, especially since Sketch doesn't provide a quick way to edit the images set as "background fill" .

Also, I was trying to use the layer name to keep the image id once it's retrieved from Unsplash. What do you think about asking for the search tags in a dialog instead of using the layer name?

nolastan commented 8 years ago
  1. The two APIs can co-exist, given how the plugin was written. If you just want the basic functionality with search terms then we use the Source API, otherwise if you want more options use the unplash.it API.
  2. What's the use case for using the layer name to store the image id? I think a dialog would make the plugin unusable if you have a dozen or so images to populate, all with different search terms.
luxlogica commented 8 years ago

This new functionality looks exciting! - but so is the capacity to get the original image's name or ID...

@nolastan There is, for sure, a very common use-case where the name or ID of the image is needed. Consider that Sketch is used my many Designers, to produce website mockups which are approved by the client, and then passed on to a Frontend Developer. If the Designer can include the original images - or at least tell the Frontend Developer where to get them - it makes everybody's job easier, and reduces the possibility of conflict with the client. To do that, they'd need the image ID.

@fhuel The capacity to search by keywords would indeed be welcome - and it may be possible to use layer names to implement BOTH functions at once: simply by using a standardised format to the layer name.

For instance, you could ask the user to specify keywords in the layer name using JSON array notation - i.e.:, [restaurant,food]. You'd have to check every selected layer name to see if it contains an array like that. If it does, perform the keyword search. If it doesn't, proceed as normal. Finally, instead of replacing the layer name with just the retrieved image ID, you could replace it with both the image ID AND the keywords array, if it had one - i.e.; [restaurant,food] id159. This would preserve the needed keywords info for future searches.

Now, that would be awesome.

I hope this suggestion helps, and I hope we'll get to see BOTH sets of functions added to this amazing plugin! Thank you for all your work - it's truly appreciated!

nolastan commented 8 years ago

Makes sense to append the ID to the layer name, but I'm not sure the JSON notation is necessary.

luxlogica commented 8 years ago

The JSON notation was just an example of a possible format - of course, you should use whatever format is most suitable for you! :-)

nolastan commented 8 years ago

Ok, I'm down to modify this PR to do it the way @luxlogica proposed. However, I don't actually see the layer renaming functionality in the plugin. @fhuel is there a branch you can push that has this?

luxlogica commented 8 years ago

Just found this - not sure whether it would be useful:

https://github.com/BohemianCoding/ExampleSketchPlugins/tree/master/Tagged%20Layers

nolastan commented 8 years ago

Yes that looks useful, thanks.

Is there anything at this point blocking this PR from being merged?

fhuel commented 8 years ago

Hey, yes, really useful link, thanks! Actually there is no need to use a json structure since the API can manage a list of strings divided with commas pretty well. But some kind of structure or separator is needed if we want to keep tags and ID together as the layer name. Just I'd avoid to make it to complicated... ideally the priority for this plugin is to let you move faster in mocking up interfaces with photographic elements.

The two APIs have basically different behaviours and capabilities, so I don't think it would be a good idea to keep both as separate actions. I believe it would be better to identify what's most useful and what can be dismissed. Of course it depends on the kind of use... but, maybe having the images blurred right away, or converted to B/W is less important than having the ability to specify tags, and we can focus on the Source API... what do you think?

Anyway I've been testing this solution with tags in my day to day work, and it seems to work quite well. Right now I still have to understand how to get the image ID back from the Source API, any idea about it ?

Also, this is a smaller detail, but it would be really nice to manage the case when the tags don't return any image. The default "not found" image becomes annoying really fast, and the reason why you get that could be not very clear, and a bit frustrating, the first times.

luxlogica commented 8 years ago

For the case when no image is returned for the search, how about using a generic, default image (that could ship with the plugin), or perhaps use one from "placehold.it"?

nolastan commented 8 years ago
  1. So given the link @luxlogica shared, we could tag the layer with the image source, and add a menu item to the plugin e.g. "View URL for image" that shows the source for the selection.
  2. Why do you think it's a bad idea to keep both API's? We don't have any way of knowing what people use. The code is already repeated rather than shared so it's not a DRY issue.
  3. I don't see any way to get the image ID from the Source API. The full API might have something but requires authentication it looks like.
  4. In my use cases, the "not found" image has been helpful so I know to use a different search term. If this is annoying in your case, we could place a random image with a message like "No results for [search term]. Placing random photo."
fhuel commented 8 years ago
  1. Sounds like a good idea!
  2. I don't think is a bad idea using both APIs . I think is a bad idea providing two completely different behaviour, at least for how the plugin is structured now. If the basic usage provides the tags functionality, the "+ options" has to do it as well, I believe.
  3. I think that the first point actually already solve this. At the end we don't need the ID if we have the link to the image, right?
  4. The problem is not the image per se, that's very useful, but the message. It makes completely sense online, but in the context of Sketch and this plugin it should ideally say something like "nothing found for those tags...".
nolastan commented 8 years ago
  1. :sunglasses:
  2. Ok, so in order to have only one API, here are the options I can think of:
    1. Don't add the layer name as search term functionality.
    2. Remove the image options functionality.
    3. Fork the project
    4. Add a third option, something like "Image from layer name" (and still have both APIs but at least it's more clear the different behavior)
  3. I'm not sure it does. You still need to get the redirect URL, and I'm not sure how to do that.
  4. The message is not ideal but I don't think it presents a problem. I guess an alternative would be to populate a random image and display a message in Sketch that nothing was found. However, not sure how to scale this to many layers. Do you have another suggestion?
luxlogica commented 8 years ago

Suggestion:

item 4. "placehold.it" is a free service that generates generic placeholder images. By default, it generates grey images with the image dimensions in the middle as a text. Both the colour of the background, as well as the text, can be customised - just by using the right URL api, just as in "unsplash.it". So, if no image was found after the search, you could use "placehold.it" to generate a placeholder 'blank' with the text "no pics found" (or something similar). :+1:

For instance, to get a 350x150 orange background placeholder with white text saying "no pics found":

http://placehold.it/350x150/ff3300/ffffff?text=no+pics+found

I believe this would work even if applied to may layers.

fhuel commented 8 years ago

@nolastan

  1. I'd go with the ii . Looks like the most sensible options right now, since both blur and B/W can be easily done directly in Sketch.
  2. You're right. I checked directly with the guys of Unsplash. It seems there is no way to get the ID from the source API. It seems it could be possible using this (unofficial) API https://github.com/naoufal/unsplash-js , but I'd keep that for the next iteration.
  3. Let's keep the default "We couldn't find that photo" image for now. @luxlogica that's a good idea for a further iteration, placehold.it is a very goo product, but I would avoid to retrieve images from the web for graphics that are very easy to create in Sketch itself .
nolastan commented 8 years ago

Hey @fhuel how's this look now?

fhuel commented 8 years ago

Cool, I have the commit almost ready, with your contribution in it. Didn't find a way to navigate to the right image on the site yet. But at this point could make more sense to publish this temporary version without that functionality.