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

Logo update #1401

Merged
merged 17 commits into from
Sep 6, 2019
Merged

Logo update #1401

merged 17 commits into from
Sep 6, 2019

Conversation

DominiqueFuchs
Copy link
Contributor

Added png exports of current logo from promo branch as preparation for subsequent changes to implement the new variant in shell implementations, build chains and packaging.

As a proposal I decided to add them besides the old one for this reason until all changes are complete. I this gets acknowledged, I'll work on implementing these files in the corresponding places.

@DominiqueFuchs
Copy link
Contributor Author

See also #22 and #810

…anch.

Decided for new naming sheme too keep old variants in repo untill all changes (also in helper repos like client-building) are done.

Signed-off-by: Dominique Fuchs <32204802+DominiqueFuchs@users.noreply.github.com>
@kesselb kesselb added 3. to review enhancement enhancement of a already implemented feature/code labels Sep 3, 2019
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Looking good!

Signed-off-by: Dominique Fuchs <32204802+DominiqueFuchs@users.noreply.github.com>
…s versions reg. the sizes) in https://github.com/KDE/extra-cmake-modules/blob/master/modules/ECMAddAppIcon.cmake

Signed-off-by: Dominique Fuchs <32204802+DominiqueFuchs@users.noreply.github.com>
…rrected filenames to not change pattern in cmake scripts

Signed-off-by: Dominique Fuchs <32204802+DominiqueFuchs@users.noreply.github.com>
Signed-off-by: Dominique Fuchs <32204802+DominiqueFuchs@users.noreply.github.com>
… ECMAddAppIcon.cmake)

Signed-off-by: Dominique Fuchs <32204802+DominiqueFuchs@users.noreply.github.com>
Signed-off-by: Dominique Fuchs <32204802+DominiqueFuchs@users.noreply.github.com>
…Icon

Signed-off-by: Dominique Fuchs <32204802+DominiqueFuchs@users.noreply.github.com>
Signed-off-by: Dominique Fuchs <32204802+DominiqueFuchs@users.noreply.github.com>
…cmake-modules/blob/master/modules/ECMAddAppIcon.cmake with modifications for nc workflow (incl. png2imagemagick)

Signed-off-by: Dominique Fuchs <32204802+DominiqueFuchs@users.noreply.github.com>
@DominiqueFuchs
Copy link
Contributor Author

Completed AFAICT for the windows buildchain & package. See pictures:
EXE
QuickAccess
SyncFolder

There are a few notable changes besides the known new files in the corresponding commit:

  • Also updated sidebar-icons for macOS
  • Updated AddAppIconMacro to also reflect new resolutions (including macOS)
  • Updated ECMAddAppIcon based on current version in KDE repo with modification to our buildflow. Included simplifications to png2ico execute_custom_command and arguments assuming png2imagemagick is present. See comment in code. Doesn't change anything to the current situation regarding this limitation. Also updated mac-image-gathering based on current guidelines by Apple. This may as well fix some retina issues as the old state was pretty outdated (in terms of used resolutions and assignments) and I saw some issue here about this a while ago.

Will report back here when I checked build and package on macOS & KDE.

@jancborchardt
Copy link
Member

What do you think @camilasan @juliushaertl @misch7?

@jancborchardt jancborchardt requested a review from misch7 September 4, 2019 16:37
@misch7
Copy link
Member

misch7 commented Sep 4, 2019

I think it's great! :-)

Works for me on Windows 👍

On my local Mac build the icons got placed in nextcloud.app > Contents > Resources > final_src.icns instead of Nextcloud.icns so I've got to try it on our build servers to be sure ;)

@DominiqueFuchs
Copy link
Contributor Author

Alright, just saw that it blows up on the drone instances, except for the debian build - strange. Output looks like the new image files were not found e.g. an empty arg call was done within the macro module.
I'll look into this.
Thanks for the comments & invite btw!

@misch7
Copy link
Member

misch7 commented Sep 4, 2019

You're welcome! =) 😸

If it helps, here's the output I got from calling:

$ git apply --check 1401.patch
error: theme/colored/nc_logo_rnd_nb_1024px.png: No such file or directory
error: theme/colored/nc_logo_rnd_nb_10px.png: No such file or directory
error: theme/colored/nc_logo_rnd_nb_128px.png: No such file or directory
error: theme/colored/nc_logo_rnd_nb_16px.png: No such file or directory
error: theme/colored/nc_logo_rnd_nb_24px.png: No such file or directory
error: theme/colored/nc_logo_rnd_nb_256px.png: No such file or directory
error: theme/colored/nc_logo_rnd_nb_32px.png: No such file or directory
error: theme/colored/nc_logo_rnd_nb_40px.png: No such file or directory
error: theme/colored/nc_logo_rnd_nb_48px.png: No such file or directory
error: theme/colored/nc_logo_rnd_nb_512px.png: No such file or directory
error: theme/colored/nc_logo_rnd_nb_64px.png: No such file or directory
error: theme/colored/nextcloud-icon-10px.png: No such file or directory
error: theme/colored/nextcloud-icon-16px.png: No such file or directory
error: theme/colored/nextcloud-icon-40.png: No such file or directory
error: theme/colored/nextcloud-icon-1024.png: No such file or directory
error: theme/colored/nextcloud-icon-128.png: No such file or directory
error: theme/colored/nextcloud-sidebar-128.png: No such file or directory
error: theme/colored/nextcloud-icon-16.png: No such file or directory
error: theme/colored/nextcloud-sidebar-16.png: No such file or directory
error: theme/colored/nextcloud-icon-24.png: No such file or directory
error: theme/colored/nextcloud-icon-256.png: No such file or directory
error: theme/colored/nextcloud-sidebar-256.png: No such file or directory
error: theme/colored/nextcloud-icon-32.png: No such file or directory
error: theme/colored/nextcloud-sidebar-32.png: No such file or directory
error: theme/colored/nextcloud-icon-48.png: No such file or directory
error: theme/colored/nextcloud-icon-512.png: No such file or directory
error: theme/colored/nextcloud-icon-64.png: No such file or directory
error: theme/colored/nextcloud-sidebar-64.png: No such file or directory

Signed-off-by: Dominique Fuchs <32204802+DominiqueFuchs@users.noreply.github.com>
…[NAME]) that changed by recent update of AddAppIcon modules. Fixes wrong naming of resource pack in build output.

Signed-off-by: Dominique Fuchs <32204802+DominiqueFuchs@users.noreply.github.com>
Signed-off-by: Dominique Fuchs <32204802+DominiqueFuchs@users.noreply.github.com>
@DominiqueFuchs
Copy link
Contributor Author

Got it. There were to related mistakes that led to linux (CI fail) and mac issues (wrong named icon pack in build output). I had to update a variable in the CMakeLists in /src/gui 7aefa5a that changed in the new ECMAddAppIconScript.

For the linux builds, I initially didn't realized possible case issues when adding the new logo files. But linux is indeed case sensitive in most (no pun intended) cases. Fixed by applying consistent naming and theme packaging e2f7947 and 6f5dcf

Mac and linux builds worked now fine on my end. Ready to merge when you are 🦊 ☺️

@camilasan camilasan added this to the 2.6 Login flow v2 milestone Sep 6, 2019
@camilasan camilasan merged commit c3b270b into nextcloud:master Sep 6, 2019
@DominiqueFuchs DominiqueFuchs deleted the logo-update branch September 6, 2019 15:34
misch7 added a commit that referenced this pull request Sep 7, 2019
New UI resources based on current https://github.com/nextcloud/promo

New:
- Icon based on #1401
- Wizard image & header image (dotted background replaces old cloud background)

Signed-off-by: Michael Schuster <michael@schuster.ms>
Copy link
Member

@misch7 misch7 left a comment

Choose a reason for hiding this comment

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

Hey @DominiqueFuchs well done, you're such a fox 🦊 ❤️

In addition to this I put together some graphics and the newly generated icon to also make your changes visible in the Windows setup wizard 😏

( Will be in RC1 (just rebuilt the installer for it) and is also in daily 2019-09-07 )

cc @camilasan @jancborchardt

misch7 added a commit that referenced this pull request Oct 17, 2019
New UI resources based on current https://github.com/nextcloud/promo

New:
- Icon based on #1401
- Wizard image & header image (dotted background replaces old cloud background)

Signed-off-by: Michael Schuster <michael@schuster.ms>
(cherry picked from commit 819a006)
Signed-off-by: Michael Schuster <michael@schuster.ms>
@misch7 misch7 mentioned this pull request Jul 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement enhancement of a already implemented feature/code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants