cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Grieve <agri...@chromium.org>
Subject Re: cordova launcher icon support https://github.com/apache/cordova-cli/pull/126
Date Wed, 12 Feb 2014 21:57:03 GMT
Another pull request came in last week:
https://github.com/apache/cordova-cli/pull/29

It adds icons & splashscreens to both iOS and Android, and even uses
imagemagick to generate them if it's available.

Still looking it over.


On Wed, Feb 12, 2014 at 4:37 PM, Andrew Grieve <agrieve@chromium.org> wrote:

>
>
>
> On Tue, Feb 11, 2014 at 3:43 PM, Brian LeRoux <b@brian.io> wrote:
>
>> Given we are the only 'widget spec' impl in use today I'm ok with
>> diverging
>> and not adding namespace confusion. Def want config to be explicit and not
>> have magical implicit mappings.
>>
>>
>> On Tue, Feb 11, 2014 at 12:24 PM, Axel Nennker <ignisvulpis@gmail.com
>> >wrote:
>>
>> > - My implementation does not use "id". Don't know what this or might
>> mean.
>>
> Found it from your test in spec/test-config.xml
>
>
>>  > - I do not want to discuss the sense of xml namespaces in this issue
>> if we
>> > can avoid it. The current template config.xml defines two namespaces and
>> > for this issue's implementation I do not want to change that. So I would
>> > not drop the widget namespace and would not support
>> > "platform"-without-prefix.
>>
>
> Having gap:platform there makes the property seem like an second-class
> maybe-not-supposed-to-be-there kind of attribute to me.
> I'd be happy to change the default template to not reference the widget
> spec and to make cordova's the default namespace if that will make your
> inner XML validator rest at-ease, but I really feel strongly against having
> XML namespaces creep in. I don't think that most devs know what they do,
> and our tools wouldn't support you changing the gap: namespace prefix.
>
>
>
>> > - I would follow the phonegap example
>> >
>> >
>> http://docs.build.phonegap.com/en_US/3.1.0/configuring_icons_and_splash.md.html#Icons%20and%20Splash%20Screens
>> > that defines e.g. "
>> >
>> > <icon src="icons/android/ldpi.png" gap:platform="android"
>> > gap:density="ldpi" />
>> >
>> > " and I would like to don't parse the icon file to infer parameters.
>> > Developers want need icons for their app on all platforms they support.
>> So
>> > they will create all of them in all polished sizes and densities.
>> > My Android implementation puts icons without cdv:density into
>> > "drawable/icon.png" regardless of width/height.
>> > What behaviour would you suggest when both lines are present in one
>> > config.xml
>> > <icon src="icon48.png" width="48" cdv:platform="android" /> // would
>> end up
>> > in drawable-mdpi by your suggestion
>> > <icon src="icon-mdpi.png" cdv:density="mdpi" cdv:platform="android" />
>> //
>> > would end up in drawable-mdpi too
>>
>
> My thinking here was that density says the same thing as size, so I would
> just not support density (or make size="mdpi" an alias for size="48").
>
>
>
>> >
>> > I think that developers know what is the "platform-way" for each
>> platform.
>> > On Android the usual way is to specify densities.
>> >
>> > - I would not use "size" because that is not w3c widget style.
>> >
>> > -- Axel
>> >
>> >
>> >
>> > 2014-02-11 20:22 GMT+01:00 Andrew Grieve <agrieve@chromium.org>:
>> >
>> > > Would love to move this along. Would like to get buy-in from others
>> > > first though.
>> > >
>> > > The proposal in this PR is to add tags like:
>> > >
>> > >     <icon id="icon" src="icon.png" />
>> > >     <icon id="logo" src="logo.png" width="255" height="255" />
>> > >     <icon src="logo-android.png" width="255" height="255"
>> > > cdv:platform="android" cdv:density="mdpi" />
>> > >
>> > > My feedback:
>> > > - What is "id" for?
>> > > - Supporting "cdv:platform" is fine, but we should also support just
>> > > "platform=". I'd be fine to drop xmlns="http://www.w3.org/ns/widgets"
>> > > from the file.
>> > > - I don't think there are any platforms that support non-square icons.
>> > > I think size="###" would be better than width= && height=
>> > > - What happens if you don't specify a size? Do we sniff it from the
>> > > png header? This might be nice as a follow-up, but I'd lean towards
>> > > making it required for the first cut.
>> > > - cdv:density seems redundant with respect to size. Icons on android
>> > > are 46px at mdpi, so the size can be used to derive the density.
>> > >
>> > >
>> > > On Mon, Feb 10, 2014 at 10:55 AM, Andrew Grieve <agrieve@chromium.org
>> >
>> > > wrote:
>> > > > He Axel, thanks for spearheading this. Will have a look shortly.
>> > > >
>> > > >
>> > > > On Mon, Feb 10, 2014 at 9:07 AM, Axel Nennker <
>> ignisvulpis@gmail.com>
>> > > wrote:
>> > > >>
>> > > >> Andrew,
>> > > >>
>> > > >> any comments to the current implementation?
>> > > >> https://github.com/apache/cordova-cli/pull/126
>> > > >>
>> > > >> Joe commented that the new class in config_parser.js named "icon"
>> > should
>> > > >> be named "Icon" but I left it as is because the other classes
are
>> > > lowercase
>> > > >> too.
>> > > >> There was another comment that namespaces in config.xml attributes
>> are
>> > > >> SchnickSchnack/chatter.
>> > > >> I think that we should use the cordava namespace if config.xml
>> > deviates
>> > > >> from the W3C widget definition.
>> > > >> These two are the only comments I got.
>> > > >>
>> > > >> I tested this on Android and FirefoxOS.
>> > > >>
>> > > >> Any chance to accept the request (at least the Android part)?
>> > > >>
>> > > >> -Axel
>> > > >>
>> > > >
>> > >
>> >
>>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message