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:37:29 GMT
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