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

New filetype icons #1124

Merged
merged 1 commit into from
Sep 9, 2015
Merged

New filetype icons #1124

merged 1 commit into from
Sep 9, 2015

Conversation

Kernald
Copy link
Contributor

@Kernald Kernald commented Aug 30, 2015

This uses the icons referenced in #1118 . As mentioned in this issue, the ldpi variant has been scaled down to 3232px (from 3636px) to keep a multiple of 16px.

Here's the result, on an xhdpi screen:
device-2015-08-30-222849

While testing this, I also noticed an issue on the file list adapter: checker background was added on PNG items, but the background wasn't reset on other items. This is visible on the screenshot above on the folder and the PDF file (I fixed this after I took the screenshot).

@rperezb rperezb added this to the 1.8-current milestone Aug 31, 2015
@masensio
Copy link

Hi @Kernald!

The bug that you detected is fixed in the PR #1114, this is in our QA process.

I think it is your first contribution, is it? In that case before to merge your PR in the master branch, we need that you will send a signed copy of the contributor agreement here http://owncloud.org/about/contributor-agreement/ to @karlitschek.

Thanks for your contribution

@Kernald
Copy link
Contributor Author

Kernald commented Aug 31, 2015

It's not my first and I already signed and sent the agreement a while ago. I can re-send it if needed. Regarding the fix, should I remove the second commit and only keep in this branch the one regarding the icons?

@masensio
Copy link

Ok, if you signed the agreement and in the past you do not need to do it again. Sorry, I didn't see you in the controibutors list.

Regarding the fix, if you want to remove the commit you can, if you'd rather leave it before merging this PR it would be updated with the new master content. As you want.

On the other hand, I have been reviewing the images you've added. We need to have the images in the resolutions: mdpi, hdpi, xhdpi and xxhdpi. Those are the most important densities right now. Mdpi is the baseline density, and I didn't see those images updated.
Please ignore ldpi, don't add more images to this folder, only update the images in this folder. Maybe this is the moment to remove this folder. @davivel, @jancborchardt what do you think?

@Kernald
Copy link
Contributor Author

Kernald commented Aug 31, 2015

Sorry you're right, I forgot to update the mdpi ones. I'll do it tonight, and remove the added ldpi images.

@Kernald
Copy link
Contributor Author

Kernald commented Aug 31, 2015

I just updated the branch:

@jancborchardt
Copy link
Member

Wow, looks great! :) Check it out @AndyScherzinger @tobiasKaminsky @stoyicker

The only thing is, as you already mentioned, the checkerboard background for the folder and PDF icon.

@masensio
Copy link

masensio commented Sep 1, 2015

Hi @Kernald,

thanks for your quickly update :)
I've reviewed again and I think that everything is right now.

So, PR ready for testing. @purigarcia

@jancborchardt
Copy link
Member

@masensio did you fix the checkerboard pattern bug? If I remember correctly, you just removed that from the file list again, right? :)

@Kernald
Copy link
Contributor Author

Kernald commented Sep 2, 2015

@jancborchardt It has been fixed in #1114 by removing the checkerboard in file list (keeping it in images background only).

@masensio
Copy link

masensio commented Sep 2, 2015

@jancborchardt, as @Kernald said, it is fix in the PR #1114.
I've added the commit 313df93 to this PR to fix it.

#1114 is merged into master now :)

@AndyScherzinger
Copy link
Contributor

Nice work! This gets us closer to a consistent oC look&feel but also adds to the material design :)

@jancborchardt
Copy link
Member

@AndyScherzinger yeah, I also though this fits very nicely with the Material design. :) high five!

@AndyScherzinger
Copy link
Contributor

@jancborchardt yeah! For having it, I locally pulled it into my FAB branch :) - Looks good for xxhdpi 😃
device-2015-09-07-144143

@rperezb
Copy link

rperezb commented Sep 7, 2015

Qa approved! @davivel or @masensio can you please review and merge,
thx

masensio pushed a commit that referenced this pull request Sep 9, 2015
@masensio masensio merged commit 84a4a08 into owncloud:master Sep 9, 2015
@masensio
Copy link

masensio commented Sep 9, 2015

PR in master branch.
Thanks @Kernald for updating the icons :)

@AndyScherzinger
Copy link
Contributor

👍 @Kernald - material design is progressing :)

@jancborchardt
Copy link
Member

Great stuff! :) When will we have a Material design release of the Android app btw? @rperezb @davitol @MTRichards?

@AndyScherzinger
Copy link
Contributor

We should at least integrate #1090 inho since:

  • the primary button coloring seems to be something that supports the UX
  • the check boxes then match the ones of the preference screen
  • material proposed paddings/margins
  • @jancborchardt's proposed file size display ;)

One thing to keep in mind is that the master already contains the new icons and thus visually matches oC 8.2 web, so maybe the releases could be around the same time.

One bug might still be in there somewhere as mentioned in #1090 but I couldn't find @tobiasKaminsky's comment anymore, so I am waiting for his reply to go on a bug hunt :)

Besides: Any input on the #1100, having the fab in this release would make the material release a big bang style wise 💃 but we can postpone that if we want to.

@Kernald Kernald deleted the new-filetype-icons branch September 9, 2015 08:21
@AndyScherzinger
Copy link
Contributor

@Kernald I created a new branch for the further icon work: https://github.com/owncloud/android/tree/new_filetype_icons - if you direct your next PR with the further icon work to this branch I can just merge it, do the coding and send a feature complete PR 😃

@davivel
Copy link
Contributor

davivel commented Sep 9, 2015

@AndyScherzinger . what future icon work?

@jancborchardt , next release, with MD stuff already merged, is programmed for mid-september. You can check the milestone: https://github.com/owncloud/android/milestones

But probably we'll need to review the date, with the current state I don't feel we're going to get it.

@ALL, could we limit the discussions in merged PRs / closed issues? There are enough open issues to continue.

Thanks,.

@AndyScherzinger
Copy link
Contributor

@davivel the screenshot of oC web @jancborchardt has posted shows two more icons (a gear and a code icon, see #1118) which are being used in the oC 8.2 web app and thus imho should also be used in the mobile app for visual consistency.

@jancborchardt
Copy link
Member

@AndyScherzinger @Kernald @davivel there’s also zip files etc – make sure all the filetypes are properly represented. ;) https://github.com/owncloud/core/tree/master/core/img/filetypes

@AndyScherzinger
Copy link
Contributor

The zip files are properly represented already :)

@davivel
Copy link
Contributor

davivel commented Sep 9, 2015

OK, thanks.

Let's follow in #1118 then.

@MTRichards
Copy link

Hey @AndyScherzinger @tobiasKaminsky - I know this is off topic, but...
It looks like we will be able to release the updated Android app in about 10-12 days. Since you have both been so involved, would you want to write technical blog(s) about the changes in that time? You can host them yourselves, or we can put them up if that is easier for you...
But these are such good visual and technical improvements across the board, and you put in work, just let me know if this is interesting.
We will make some noise about the new releases and link to your blogs if you want to do so. No pressure, whatever you want to do here is ok with me.

@AndyScherzinger
Copy link
Contributor

Hey @MTRichards,
sure, in general.
I do not run a blog atm and would rather leave the putting up / hosting with you. I can't promise you any articles (due to possible time constraint on my side) but I will try to do a write up of the changes been done for material in this first material release and probably about the (possible) changes coming in the future with the other material PRs I opened. So it will give people an impression of what has been done (probably a rather technical article) and tease what to expect in the future :)

In case I would do a write up what would be the desired file format? (I most likely will include several, external links to google design and other blog posts, if that is okay :)

@MTRichards
Copy link

I understand, time constraints being what they are - not a problem. We also take short, long or in the middle - up to you and your time.

Any format that works for you is fine. We will probably turn it into a blog on our .org site then, and reference that in other posts.
@jospoortvliet FYI, if Andy chooses to do a blog, would need help to publish on .org.

@AndyScherzinger
Copy link
Contributor

@jospoortvliet @MTRichards I will try to write a post this saturday. Any input on the content?

I would write about the material design obviously and would most likely focus on the development aspects behind this... besides mentioning the visual things that changed and would also like to write a couple of lines motivating other to join the development of the app and the eco system in general since I am enjoying this great experience with everyone here in the community and also the quite low entrance barrier to contribute to this project. So this last issue would also be something it would like to mention :)

Blog post on .org sound absolutely fine to me 👍

@jancborchardt
Copy link
Member

@AndyScherzinger cool stuff! :) Would be good to collaborate with @tobiasKaminsky to get a joint blog post out. :) Or maybe @tobiasKaminsky you want to focus on a different topic in a separate blog post? We can definitely make this a more regular thing and not just a one-off, maybe @Kernald or @stoyicker also want to blog in the future? :)

@Kernald
Copy link
Contributor Author

Kernald commented Sep 10, 2015

@jancborchardt Why not, I already maintain a blog, I'm pretty sure I could have posts interresting for the OwnCloud community!

@MTRichards
Copy link

I would write about the material design obviously and would most likely focus on the development aspects behind this... besides mentioning the visual things that changed and would also like to write a couple of lines motivating other to join the development of the app and the eco system in general since I am enjoying this great experience with everyone here in the community and also the quite low entrance barrier to contribute to this project. So this last issue would also be something it would like to mention :)

All of that sounds great, go for it!

@stoyicker
Copy link
Contributor

@jancborchardt Unfortunately for me, it is long that I can barely find time to commit to open source projects and, as you may have noticed, even less to ownCloud, so blogging in the long-term future is not a thing for me. However, should this be demanded, I could possibly write about the great experience that it has been as a "farewell".

@jospoortvliet
Copy link

@AndyScherzinger and everybody else - write a bit about your view on things. How it was, what was hard, what was fun, what was easy, anything. And what you did etc.

Don't worry about spelling, grammar and so on - or format, any format is OK. Well, try not to send my binaries or utf 9.3 (?), but flat text or odt or markdown or html is all fine. ;-)

I'll be more than happy to put it in a nice looking blog post for .org.

mail my at jos @ the owncloud.com server pls.

Great work, all!

@AndyScherzinger
Copy link
Contributor

@jospoortvliet You got mail! :)

@jospoortvliet
Copy link

I've seen it and I'll make sure we have a nice blog by the time we get the new Android client out. Unfortunately I'm very busy at the moment but it will come ;-)

@tobiasKaminsky
Copy link
Contributor

@MTRichards Sorry, but I am currently to busy to write a blog.
But sometime in the future, maybe for a upcoming release I can write something

@MTRichards
Copy link

Ok, no problem. Just didn't want you to be left out. We will ask again!

@jospoortvliet
Copy link

@tobiasKaminsky we've got a nice blog now, with text from @AndyScherzinger (who wrote an epic overview of what's new with material design) and thoughts from @stoyicker as well. So we're good.

For next time, yeah, it'd be great if you could share something of course. Note that I don't need much - just a brain dump, raw thoughts with some links and I'm happy to turn them in something pretty!

@jesmrec jesmrec added File list and removed Image labels Mar 27, 2023
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.