Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Ugly Icon #27

Closed
neilt6 opened this issue Jul 26, 2018 · 26 comments
Closed

Ugly Icon #27

neilt6 opened this issue Jul 26, 2018 · 26 comments

Comments

@neilt6
Copy link

neilt6 commented Jul 26, 2018

Is there a reason this version is using an ugly poorly-scaled version of the Notepad++ icon? If not, I wouldn't mind rebuilding the visual assets and submitting a pull request.

@jakevis
Copy link
Member

jakevis commented Jul 26, 2018 via email

@neilt6
Copy link
Author

neilt6 commented Jul 26, 2018

Ok, I've submit a pull request for your review. I used the asset generator in Visual Studio to spit out assets for all of the different icon sizes in the Notepad++ ICO, then selected the best versions to produce the final set.

@jakevis
Copy link
Member

jakevis commented Jul 26, 2018

So just merged it in and built it to test - this is what I have:
image

And when pinned to start:
image

@jakevis
Copy link
Member

jakevis commented Jul 26, 2018

@jakevis
Copy link
Member

jakevis commented Jul 26, 2018

@neilt6 - there were 14 files in assets that wernt updated it seems (were missed in the merge / VS didn't create them). Any chance you can generate better quality copys of these:

NightRise.NotepadWrapped/PackageFiles/Assets/Square310x310Logo.scale-100.png NightRise.NotepadWrapped/PackageFiles/Assets/Square310x310Logo.scale-125.png NightRise.NotepadWrapped/PackageFiles/Assets/Square310x310Logo.scale-150.png NightRise.NotepadWrapped/PackageFiles/Assets/Square310x310Logo.scale-200.png NightRise.NotepadWrapped/PackageFiles/Assets/Square44x44Logo.targetsize-16_altform-unplated.png NightRise.NotepadWrapped/PackageFiles/Assets/Square44x44Logo.targetsize-24_altform-unplated.png NightRise.NotepadWrapped/PackageFiles/Assets/Square44x44Logo.targetsize-256_altform-unplated.png NightRise.NotepadWrapped/PackageFiles/Assets/Square44x44Logo.targetsize-32_altform-unplated.png NightRise.NotepadWrapped/PackageFiles/Assets/Square44x44Logo.targetsize-48_altform-unplated.png NightRise.NotepadWrapped/PackageFiles/Assets/Square71x71Logo.scale-100.png NightRise.NotepadWrapped/PackageFiles/Assets/Square71x71Logo.scale-125.png NightRise.NotepadWrapped/PackageFiles/Assets/Square71x71Logo.scale-150.png NightRise.NotepadWrapped/PackageFiles/Assets/Square71x71Logo.scale-200.png NightRise.NotepadWrapped/PackageFiles/Assets/Square71x71Logo.scale-400.png

Ill see what I can do as well, but mostly focusing on getting an update into the store for the latest bug fix build.

@neilt6
Copy link
Author

neilt6 commented Jul 26, 2018

@jakevis Sorry, I'll look into this ASAP. It's probably the manifest or resources.pri file, I overlooked updating those.

@neilt6
Copy link
Author

neilt6 commented Jul 26, 2018

@jakevis As for the missing sizes, I don't think they're required. At least Visual Studio doesn't generate them by default, and none of the apps I've published to the store have them.

@jakevis
Copy link
Member

jakevis commented Jul 26, 2018

no issues at all; thanks for your help so far - it will make quite a number of people a lot happier im sure

@neilt6
Copy link
Author

neilt6 commented Jul 26, 2018

@jakevis Ok, I think I've found the problem. Visual Studio names the Square310x310Logo and Square71x71Logo assets LargeTile and SmallTile respectively. Updating the manifest should do the trick.

@jakevis
Copy link
Member

jakevis commented Jul 26, 2018

@neilt6 - ok excellent - let me do that now and re-build, thanks for checking

@neilt6
Copy link
Author

neilt6 commented Jul 26, 2018

@jakevis Also, a minor critique while we're editing the manifest: the DisplayName should probably be "Notepad++" not "NotePad++". :)

jakevis added a commit that referenced this issue Jul 26, 2018
jakevis added a commit that referenced this issue Jul 26, 2018
jakevis added a commit that referenced this issue Jul 26, 2018
working on #27
@jakevis
Copy link
Member

jakevis commented Jul 26, 2018

Ok - so the component looks like this in the manifest now:

  <uap:VisualElements DisplayName="Notepad++" Description="Notepad++ is a free (as in free speech and also as in free beer) source code editor and Notepad replacement that supports several languages. Running in the MS Windows environment, its use is governed by GPL License." BackgroundColor="transparent" Square150x150Logo="Assets\Square150x150Logo.png" Square44x44Logo="Assets\Square44x44Logo.png">
    <uap:DefaultTile Wide310x150Logo="Assets\Wide310x150Logo.png" Square310x310Logo="Assets\LargeTile.png" Square71x71Logo="Assets\SmallTile.png">
      <uap:ShowNameOnTiles>
        <uap:ShowOn Tile="square150x150Logo" />
        <uap:ShowOn Tile="wide310x150Logo" />
        <uap:ShowOn Tile="square310x310Logo" />
      </uap:ShowNameOnTiles>
    </uap:DefaultTile>
  </uap:VisualElements>

@neilt6
Copy link
Author

neilt6 commented Jul 26, 2018

@jakevis Looks good to me, fingers crossed!

@jakevis
Copy link
Member

jakevis commented Jul 26, 2018

no luck - atleast not on my machine... Ill go swap the names of the files around just incase there is something stupid in there.

@neilt6
Copy link
Author

neilt6 commented Jul 26, 2018

@jakevis Have you tried rebuilding resources.pri?

@neilt6
Copy link
Author

neilt6 commented Jul 26, 2018

@jakevis I just checked resources.pri in the package you sent me, and it looks like it's stale. I still see references to the old filenames in there.

jakevis added a commit that referenced this issue Jul 26, 2018
@jakevis
Copy link
Member

jakevis commented Jul 26, 2018

rebuilding that takes a little more effort.. since I need to go back to the desktop packaging tool and containers... [and ill need my other machine]. Appveyor can compile the exe and dll.. and run makeappx, but anything more than that need to go back to a local build...

let see how this build looks - it might work if there are just references in there.

@neilt6
Copy link
Author

neilt6 commented Jul 26, 2018

@jakevis Renaming the files might fix the issue at hand, but the extra assets might not be used until resources.pri is rebuilt.

@neilt6
Copy link
Author

neilt6 commented Jul 26, 2018

You should be able to rebuild resources.pri just by using makepri from the visual studio command prompt:

makepri new /pr "YOUR_PACKAGE_ROOT" /cf "SOME_DIRECTORY\priconfig.xml"

priconfig.zip

@jakevis
Copy link
Member

jakevis commented Jul 26, 2018

yep - just found that here: https://docs.microsoft.com/en-us/windows/uwp/porting/desktop-to-uwp-manual-conversion

Also... https://docs.microsoft.com/en-us/windows/uwp/porting/desktop-to-uwp-manual-conversion#optional-add-target-based-unplated-assets

Target-based assets are for icons and tiles that appear on the Windows taskbar

But lets see if this fixes it first (and i might be able to script that build in appveyor)

@neilt6
Copy link
Author

neilt6 commented Jul 26, 2018

@jakevis The optional unplated assets are there, Visual Studio just uses a slightly different naming convention for them: altform-unplated_targetsize-XX instead of targetsize-XX_altform-unplated. Originally I thought Square310x310Logo and Square71x71Logo were sizes I hadn't provided, but it turns out Visual Studio just uses a different naming convention for these as well. I blame Microsoft...

jakevis added a commit that referenced this issue Jul 26, 2018
@jakevis
Copy link
Member

jakevis commented Jul 26, 2018

oops - that commit shouldn't have closed... and yes, this naming is a PITA ;) wish VS + Desktop converter used the same naming ;)

@jakevis jakevis reopened this Jul 26, 2018
@neilt6
Copy link
Author

neilt6 commented Jul 26, 2018

@jakevis Sorry this turned into a bit of a hassle, I should have paid more attention to the naming when I created my original PR.

@jakevis
Copy link
Member

jakevis commented Jul 26, 2018

all good - I learned some new things. am backing out the pri thing for the moment though.. that needs more testing, and I might need to restructure the build a little bit

@jakevis
Copy link
Member

jakevis commented Jul 26, 2018

ok - https://ci.appveyor.com/api/buildjobs/m1spagh4d03gl956/artifacts/NightRise.NotepadWrapped_signed.x64.appx

its looking a lot better to me 👍

@jakevis
Copy link
Member

jakevis commented Jul 26, 2018

Thanks for your help with this one @neilt6

  • update has now been pushed to the store. With luck we will see it start rolling out in 3-4 business days.

@jakevis jakevis closed this as completed Jul 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants