owncloud / android

:phone: The ownCloud Android App
GNU General Public License v2.0
3.81k stars 3.05k forks source link

Use new filetype icons #1118

Closed jancborchardt closed 9 years ago

jancborchardt commented 9 years ago

From core master: https://github.com/owncloud/core/tree/master/core/img/filetypes

Preview: filetype icons new

They are 16*16px but of course scale them up. Please only in multiples of 16px, for example 48px.

This should be done in sync with the release of 8.2 server. cc @rperezb @MTRichards @davivel @masensio

ggdiez commented 9 years ago

Hey @jancborchardt

These new icons looks nice. :+1:

I can not wait to change the current ones for these.

jancborchardt commented 9 years ago

cc @tobiasKaminsky @AndyScherzinger @Stoyicker @Kernald too :)

AndyScherzinger commented 9 years ago

Nice! This should simply be replacing the actual png files with these and that's it :)

Kernald commented 9 years ago

What about the ldpi version? Currently, the images are 36*36 px - which is not a multiple of 16. Should I upscale them to 48 px?

jancborchardt commented 9 years ago

Hm, damn. It’s pixel-perfect at 16_16. Maybe then it’s better to just use 32_32 instead of 36?

ghost commented 9 years ago

@AndyScherzinger can you add these to your material design work for android? when is material design going to be merged? can't wait :)

Kernald commented 9 years ago

@JasperWeiss I generated all the icons in the right size, I'm just waiting for an answer regarding the ldpi size to open a PR.

jancborchardt commented 9 years ago

@Kernald as I said: Use 32 * 32 instead of 36. :)

Kernald commented 9 years ago

Alright, I'm not at home right now, I'll open the PR later today.

jancborchardt commented 9 years ago

Great stuff, thank you! :)

rperezb commented 9 years ago

thanks @Kernald ! @masensio will review it :smile:

AndyScherzinger commented 9 years ago

should further icons be added? The screenshot of the webapp shows the gear (for bin) and the "<>" for css. This would also be something that could potentially be added to the android app :)

Kernald commented 9 years ago

For reference, if additional icons are planned: the mime-type/icon equivalency table from core is here.

AndyScherzinger commented 9 years ago

For Android the icon calculation can be found here: https://github.com/owncloud/android/blob/master/src/com/owncloud/android/utils/DisplayUtils.java#L160

AndyScherzinger commented 9 years ago

@Kernald could you add the other two? Just the icon work, I can work on the bindings :) - Would this be something you would like to be done @rperezb / @jancborchardt ?

Kernald commented 9 years ago

I can generate the icons if needed, yes, that's not a problem. Just ping me if they're needed!

AndyScherzinger commented 9 years ago

Sounds great, just waiting for the others' reply :)

AndyScherzinger commented 9 years ago

@jancborchardt @rperezb what do you think? Adding the other icons would make sense imho since they will be introduced by oC 8.2 on the web and the App would display another icon in case of the new icons (gear/code) which would lead to an inconsistency.

AndyScherzinger commented 9 years ago

If @Kernald adds them to his PR #1124 I can make the necessary changes to the code afterwards in a separate PR :)

jancborchardt commented 9 years ago

Well, the icons should be used for the same filetypes as in the web interface. For example, the icon-code is to be used for all kinds of code filetypes, not only for CSS. See the mimetype list table @Kernald posted: https://github.com/owncloud/core/blob/master/core/js/mimetypelist.js

AndyScherzinger commented 9 years ago

@jancborchardt I am aware of that :smile: I would change the Android code according to the web code.

@Kernald can you add the other icons that would be great! :+1:

Kernald commented 9 years ago

Sure, I'll generate them tonight

AndyScherzinger commented 9 years ago

@Kernald sweet! Thanks a lot!

rperezb commented 9 years ago

hey,

yes, this is a great idea. @AndyScherzinger a new pr makes a lot of sense! waiting for it, in the meantime, the one for the new icons is merged https://github.com/owncloud/android/pull/1124

Kernald commented 9 years ago

I didn't have any time to generate the remaining icons yesterday, I'll try to do this tonight, sorry about that.

AndyScherzinger commented 9 years ago

Sounds great @Kernald - no worries about the timeline :) anytime you like is fine for me!

@rperezb I will open up a new PR with an implementation as soon as the new icons are present and I can start rewriting the icon choosing code and put everything in a new master-based branch.

@rperezb Should I then also open up a new issue or just have any discussion etc. in the PR itself?

davivel commented 9 years ago

Let's keep this issue open for the additional icons.

Thanks everybody for so awesome work.

Kernald commented 9 years ago

A new PR (#1147) has been opened with additional icons ;-)

jancborchardt commented 9 years ago

So we can close this @Kernald? :)

jancborchardt commented 9 years ago

(Ah no, the logic is missing. :)

AndyScherzinger commented 9 years ago

@jancborchardt how smart can my detection algorithm be? I ask this since I would incorporate two things from the web app:

Thus match the type according to the way the web app does it, calculate the mimetype for the file extensions based on the web app's definition :)

jancborchardt commented 9 years ago

@AndyScherzinger this is the most current list: https://github.com/owncloud/core/blob/master/core/js/mimetypelist.js

AndyScherzinger commented 9 years ago

That's the one I referred to ;)

jancborchardt commented 9 years ago

@AndyScherzinger I know. :) I’m not sure about the other one cc @rullzer

AndyScherzinger commented 9 years ago

imho, the other is used to determine the mime type, based on the file extension :)

AndyScherzinger commented 9 years ago

Looking at the android code this change will have an impact since the web app code (as of now) detects certain types the android app would after my update. On the other hand the Android app also detects types, like rtf, die web app doesn't... at least not from the above mentioned files.

rullzer commented 9 years ago

yes, the mimetypemapping is uses to map extentions to mimetypes. Aliases ( https://github.com/owncloud/core/blob/master/config/mimetypealiases.dist.json ) are used to map multiple mimetypes to one (e.g.. all images types).

And then here is the logic to go from a mimetype to the right image: https://github.com/owncloud/core/blob/master/core/js/mimetype.js

AndyScherzinger commented 9 years ago

https://github.com/owncloud/android/blob/master/src/com/owncloud/android/utils/DisplayUtils.java#L162 does the mapping on android, which shows a hacky one for "application" which might simply be due to android (as in operating system) failing to detect some cases and then defaults to application/octet-stream and then tries to determine the correct icon based on the file extension. So for this I would simple add further mappings to the file extension map and then simply drop the whole implementation, since maintaining a map seems way easier to me than maintaining an algorithm or some if/else cascade :)

I'll post the added values later, so they can be added to the web app to, which is what I would recommend for visual consistency.

AndyScherzinger commented 9 years ago

@rullzer looking at the js code, how do you detect the pdf?

AndyScherzinger commented 9 years ago

@rullzer running a matcher test for the code leaves the following pair without a nice icon, which seems odd, since I don't think the web app doesn't match mp3, pdf, and oders like that. The list is "extension" / "mimetype" and no separate icon thus always showing the file.

kra / application/x-krita mpg / video/mpeg tif / image/tiff ogv / video/ogg sr2 / image/x-dcraw mpo / image/jpeg 3gp / video/3gpp oga / audio/ogg iiq / image/x-dcraw webm / video/webm blend / application/x-blender ogg / audio/ogg xrf / image/x-dcraw tiff / image/tiff keynote / application/x-iwork-keynote-sffkey arw / image/x-dcraw markdown / text/markdown wmv / video/x-ms-wmv mpeg / video/mpeg mov / video/quicktime m4v / video/mp4 sgf / application/sgf impress / text/impress bin / application/x-bin ical / text/calendar gif / image/gif dv / video/dv rw2 / image/x-dcraw orf / image/x-dcraw conf / text/plain cnf / text/plain md / text/markdown numbers / application/x-iwork-numbers-sffnumbers srf / image/x-dcraw dng / image/x-dcraw nef / image/x-dcraw pages / application/x-iwork-pages-sffpages jpg / image/jpeg m2t / video/mp2t png / image/png c++ / text/x-c++src pef / image/x-dcraw jps / image/jpeg mt2s / video/MP2T reveal / text/reveal mts / video/MP2T jpeg / image/jpeg mobi / application/x-mobipocket-ebook avi / video/x-msvideo key / application/x-iwork-keynote-sffkey opus / audio/ogg odg / application/vnd.oasis.opendocument.graphics cpp / text/x-c++src cr2 / image/x-dcraw odf / application/vnd.oasis.opendocument.formula mdwn / text/markdown k25 / image/x-dcraw mkv / video/x-matroska rtf / application/rtf msi / application/x-msi bmp / image/bmp flv / video/x-flv flac / audio/flac mkd / text/markdown exe / application/x-ms-dos-executable vcf / text/vcard mef / image/x-dcraw ics / text/calendar erf / image/x-dcraw mp4 / video/mp4 vob / video/dvd mp3 / audio/mpeg wav / audio/wav txt / text/plain mdown / text/markdown bpg / image/bpg kdc / image/x-dcraw vcard / text/vcard dcr / image/x-dcraw raf / image/x-dcraw

davivel commented 9 years ago

I read too fast, sorry if I missed something important.

The general rule should be * incremental * improvement:

AndyScherzinger commented 9 years ago

I wasn't going to drop android app functionality, just posting the list for @rullzer to decide on what he wants to do with it. I will on the other hand add all mapping neccessary/missing on the android side :)

So it is incremental in a functional way, but the detection is a complete rewrite of today's code.

davivel commented 9 years ago

Nice :+1:

AndyScherzinger commented 9 years ago

One thing I will add due to the amount of extension/mimetypes not being is a simple mapping for a certain amount of main types:

in contrast the following main types won't me mapped as a fallback and thus be shown as file in case the complete mime type couldn't be matched:

AndyScherzinger commented 9 years ago

Mapping suggested I am going to implement for the not yet mapped ones (from the comments above):

if not explizitly stated it defaults based on the main type

kra / application/x-krita mpg / video/mpeg tif / image/tiff ogv / video/ogg sr2 / image/x-dcraw mpo / image/jpeg 3gp / video/3gpp oga / audio/ogg iiq / image/x-dcraw webm / video/webm blend / application/x-blender ogg / audio/ogg xrf / image/x-dcraw tiff / image/tiff keynote / application/x-iwork-keynote-sffkey --> presentation arw / image/x-dcraw markdown / text/markdown wmv / video/x-ms-wmv mpeg / video/mpeg mov / video/quicktime m4v / video/mp4 sgf / application/sgf --> defaulting to file impress / text/impress bin / application/x-bin --> defaulting to file ical / text/calendar --> calendar gif / image/gif dv / video/dv rw2 / image/x-dcraw orf / image/x-dcraw conf / text/plain cnf / text/plain md / text/markdown numbers / application/x-iwork-numbers-sffnumbers --> spreadsheet srf / image/x-dcraw dng / image/x-dcraw nef / image/x-dcraw pages / application/x-iwork-pages-sffpages --> doc jpg / image/jpeg m2t / video/mp2t png / image/png c++ / text/x-c++src --> code pef / image/x-dcraw jps / image/jpeg mt2s / video/MP2T reveal / text/reveal mts / video/MP2T jpeg / image/jpeg mobi / application/x-mobipocket-ebook avi / video/x-msvideo key / application/x-iwork-keynote-sffkey --> presentation opus / audio/ogg odg / application/vnd.oasis.opendocument.graphics --> defaulting to file cpp / text/x-c++src --> code cr2 / image/x-dcraw odf / application/vnd.oasis.opendocument.formula --> defaulting to file mdwn / text/markdown k25 / image/x-dcraw mkv / video/x-matroska rtf / application/rtf --> doc msi / application/x-msi --> application bmp / image/bmp flv / video/x-flv flac / audio/flac mkd / text/markdown exe / application/x-ms-dos-executable --> application vcf / text/vcard --> vcard mef / image/x-dcraw ics / text/calendar --> calendar erf / image/x-dcraw mp4 / video/mp4 vob / video/dvd mp3 / audio/mpeg wav / audio/wav txt / text/plain mdown / text/markdown bpg / image/bpg kdc / image/x-dcraw vcard / text/vcard --> vcard dcr / image/x-dcraw raf / image/x-dcraw

AndyScherzinger commented 9 years ago

Here is the test run for file extensions, pretty verbose....

kra / file mpg / movie woff / image tif / image potx / presentation ogv / movie sr2 / image mpo / image swf / app pptx / presentation 3gp / movie pot / presentation dot / document oga / sound iiq / image webm / movie php / code blend / file ogg / sound xrf / image tiff / image keynote / presentation xlam / spreadsheet arw / image markdown / document wmv / movie mpeg / movie gz / archive sh-lib / code pps / presentation cdr / image ppt / presentation mov / movie m4v / movie sgf / file pl / code impress / document ppa / presentation tar / archive csv / spreadsheet bin / file ical / calendar gif / image css / code dv / movie yaml / code pfb / image rw2 / image orf / image conf / document deb / archive cnf / document md / document numbers / spreadsheet epub / document srf / image yml / code dng / image apk / archive 7z / archive nef / image c / code ttf / image pages / document h / code jpg / image m2t / movie png / image c++ / code pef / image doc / document potm / presentation jps / image bash / code mt2s / movie xls / spreadsheet reveal / document mts / movie xlsx / spreadsheet jpeg / image pdf / pdf js / code docm / document mobi / file xla / spreadsheet eot / image psd / image xlsm / spreadsheet otf / image xlsb / spreadsheet dotx / document avi / movie htm / code key / presentation opus / sound xltx / spreadsheet odg / file mdb / file gzip / archive cpp / code cr2 / image odf / file mdwn / document k25 / image mkv / movie odp / presentation xml / code cb7 / document sh / code rtf / document ods / spreadsheet odt / document msi / app bmp / image eps / image xltm / spreadsheet xcf / image flv / movie flac / sound tex / document mkd / document xlt / spreadsheet exe / app cc / code vcf / vcard ppam / presentation mef / image ics / calendar erf / image mp4 / movie vob / movie mp3 / sound wav / sound hh / code rar / archive txt / document cbtc / document json / code mdown / document accdb / file ppsm / presentation zip / archive ppsx / presentation cbt / document svg / image cbr / document bpg / image cvbdl / document kdc / image cbz / document vcard / vcard dcr / image ai / image py / code cba / document docx / document html / code tgz / archive raf / image pptm / presentation ps / image

AndyScherzinger commented 9 years ago

@davivel @jancborchardt @Kernald @tobiasKaminsky @masensio - please see #1149 for the PR with the new implementation + icons

example screenshot showing some of the additional icons now incorporated: device-2015-09-10-180148

AndyScherzinger commented 9 years ago

@rperezb just wanted to hint that #1149 which is to some extend part of #1118 is not planned for 1.8 (which is fine I guess), so the additional 8.2 icons (calendar, contact, code) won't be in this release, just the 8.2 icons for the ones already been present in 1.7.2 (but now with the new design).

Like I said, not a problem, but I wanted to mention it just in case, since we split the issue into two PRs being: #1124 and #1149 - the later definitely would need some testing since it also made code changes necessary due to the new/additional icons and a new way to decide which icon to uses, aligned with the web app's implementation.

jancborchardt commented 9 years ago

Similar to iOS, the filetype icons are now stuck too far to the left. Can you move them to the right a bit so they are centered between the left edge and the filename? :)

rullzer commented 9 years ago

@AndyScherzinger I'm think you are missing something. Lets walk trough the pdf example.

If you look at https://github.com/owncloud/core/blob/master/config/mimetypemapping.dist.json you see that for example

"pdf": ["application/pdf"]

So the pdf extention is mapped to the application/pdf mimetype.


Now https://github.com/owncloud/core/blob/master/core/js/mimetypelist.js is a cache we have. But the files part is basically all the icons we have.


Now the logic is in https://github.com/owncloud/core/blob/master/core/js/mimetype.js More specifically in https://github.com/owncloud/core/blob/master/core/js/mimetype.js#L41-L60.

This function takes the mimetype (application/pdf). It replaces the / with - (application-pdf). And then starts to look for a matching icon. First we check for some special cases. Then we check (in order) for

If none of those mach we return the default icon. But there is an icon application-pdf. So we use that one.


Another short example. .mp3 is mapped to audio/mpeg. Now there is no audio-mpeg icon. But there is an audio icon.


I hope this clears it up a bit. But kick me if you need more info.