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

CB-2606 add icon support #126

Closed
wants to merge 54 commits into from
Closed

CB-2606 add icon support #126

wants to merge 54 commits into from

Conversation

AxelNennker
Copy link

In the latest commits I removed the restriction that icons are in specific directories below project_dir.
Now icon_src is taken as the source without prepending specific directories

};

icon.prototype = {
add:function(elt) {
Copy link
Contributor

Choose a reason for hiding this comment

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

add() and remove() don't seem to be used at all (other than in tests). I think they can be deleted.

Copy link
Author

Choose a reason for hiding this comment

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

They are not used. I included them because the other "classes" (access, preference) have the same methods.
Are they used for e.g. preference somewhere? If yes, then I guess the same reasons to have them for preference apply to icon as well.

@sgrebnov
Copy link
Member

@AxelNennker, could you please rebase your code on top of the apache master? I don't see, for example, config_parser.js anymore. It was renamed plus there are new functionality here which may be useful.

@AxelNennker
Copy link
Author

Re-implemented the "magic" for android. Typical sizes on Android are recognized and used. A default icon is copied to all res/drawable-*/ directories.
A simple way to get launcher icons without xml namespace juggling or custom element in config.xml.

@cmarcelk
Copy link
Contributor

In a broad sense this looks reasonable.

In the config.xml, why is there width and height attributes when using cdv:platform="android" and cdv:density="mdpi"? Is hope to reuse the same element for iOS and other non-Android platforms? But doing so doesn't seem like it is meant to be compatible with cdv:platform="android". Or is it meant to be an alternate way of specifying cdv:density="mdpi" in the Android case?

I don't see where your code uses the "id" attribute in the element. What is that element supposed to do? And what happens when it's not present in the element?

It would be great to see some corresponding docs for this (i.e., cordova-docs). Those docs would describe from the end-users view how they would consume this new capability.

When it inevitably comes time to do this for splash screens, will this icon approach work for splash screens?

sgrebnov added a commit to sgrebnov/cordova-docs that referenced this pull request Apr 3, 2014
1. iOS, Andoroid, FirefoxOS, Windows8, WP8
2. Related PR with support apache/cordova-cli#126
sgrebnov added a commit to sgrebnov/cordova-docs that referenced this pull request Apr 3, 2014
1. iOS, Andoroid, FirefoxOS, Windows8, WP8
2. Related PR with support apache/cordova-cli#126
@sgrebnov
Copy link
Member

sgrebnov commented Apr 7, 2014

Something goes wrong with this branch and the pull request. Our commits must be on top of apache/master head and we should see only those commits. Right now there is unrelated work here.

As simplest solution propose to clean history and create new single commit.
http://stackoverflow.com/a/6258664/255654

Axel, you can force push my branch if you want/ok
https://github.com/MSOpenTech/cordova-cli/tree/CB-2606

@sgrebnov
Copy link
Member

Hi @AxelNennker, I'm just trying to get this merged ASAP so I've opened a separate PR which constants the same changes but re-based and could be merged. Feel free to close the new PR if you feel we should better to complete organizing this one. Thx!

#166

asfgit pushed a commit to apache/cordova-docs that referenced this pull request Apr 24, 2014
1. iOS, Andoroid, FirefoxOS, Windows8, WP8
2. Related PR with support apache/cordova-cli#126
@AxelNennker
Copy link
Author

Merged in as PR#166
#166
by Bryan.
This closes CB-2606

@rohitagg28
Copy link

Hi
I am building the cordova application and I want change the default icon of the app .
I am using android platform and I have put all my icons sizes into res folder in www of cordova project.
So can you suggest the way how my platform will map those images from www folder.

@AxelNennker
Copy link
Author

https://cordova.apache.org/docs/en/3.5.0/config_ref_images.md.html#Icons%20and%20Splash%20Screens

<platform name="android">
<icon src="res/android/ldpi.png" density="ldpi" />
<icon src="res/android/mdpi.png" density="mdpi" />
<icon src="res/android/hdpi.png" density="hdpi" />
<icon src="res/android/xhdpi.png" density="xhdpi" />
</platform>

res is NOT relative to the www directory but to the project directory, which is one above www.
Do not put app-launcher icons into the www folder.

You can use "cordova platform add android -d" to view (error) messages.

@rohitagg28
Copy link

Yes I have follow the above procedure by putting res folder outside www and also follow the cordova documentation but still the icon doesn't change for my application .
I want to ask that when I add platform by 'cordova platform add android' command , shall I have to make any changes in platform-> android folder also because the icons in android->res->drawable folder are not changing and showing the defaults ones.

@AxelNennker
Copy link
Author

This is a 3.5 feature. Are you using the latest cordova version?
I suggest to take further discussion to stackoverflow or contact my directly.

@rohitagg28
Copy link

Thanks. The problem get solved by using the updated version.

@rohitagg28
Copy link

Hi I am integrating the icon for my app as using the trailing code but I am unable to add splashscreen to my app ,can you suggest anything.

     <icon src="/res/android/icon-36-ldpi.png"  density="ldpi" />
<icon src="/res/android/icon-48-mdpi.png"  density="mdpi" />
<icon src="/res/android/icon-72-hdpi.png"  density="hdpi" />
<icon src="/res/android/icon-96-xhdpi.png" density="xhdpi" />

@rohitagg28
Copy link

Hi I want to ask that can I add custom splashscreen to my project without making any changes in platforms folder .

@sgrebnov
Copy link
Member

Pls take a look here and here. We are working on new proposal for splash screen support which will make our live easier.

@rohitagg28
Copy link

Yes I have used these methods but these all suggest to replace the image into platforms folder that means to add manually but I want to add splashscreen without making changes to platforms folder as we can add app icon in 3.5 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants