Listview: Added option for the default list icon #5018

Merged
merged 5 commits into from Oct 9, 2012

Conversation

Projects
None yet
4 participants
Contributor

jasondscott commented Sep 17, 2012

No description provided.

Jason Scott added some commits Sep 17, 2012

Jason Scott Listview: Added option for the default list icon
Added an option for the default list icon, instead of being hard-coded
to 'arrow-r'
a485ae2
Jason Scott Docs: Added indicatorIcon to the listView docs options a243635
Member

jaspermdegroot commented Sep 18, 2012

hi @jasondscott !

@MauriceG and I already had a nice discussion about making it possible to set a default icon for listviews. See #4582.

I think we should keep things simple for users and not add new options if not really necessary. So I suggest to stick to one option "icon" with the following possibilities:

default = "arrow-r"

  1. [global] override the default for all listviews by using the $.mobile object
  2. [UL/OL] override the default or global with data-icon on a listview OR with the option icon if that listview is created programmatically
  3. [LI] override any of the above for a list item with data-icon on that specific list item

This could be done with the following additions / changes:

$.widget( "mobile.listview", $.mobile.widget, {
    options: {
        icon: "arrow-r",
    },


    refresh: function( create ) {
        var ...,
            listIcon = $list.jqmData( "icon" ) || o.icon,


            if ( a.length && !isDivider ) {
                    var itemIcon = item.jqmData( "icon" );   /* replaces: icon = item.jqmData( "icon" ); */

                    if ( a.length > 1 ) {
                        icon = false;
                    } else {
                        icon = itemIcon || listIcon;
                    }

                    item.buttonMarkup({
                        icon: icon,
                    });
Contributor

toddparker commented Sep 18, 2012

Thanks for the PR @jasondscott. Been hashing this out in IRC and I agree that the best approach would to re-use the data-icon attribute and just allow it to be set at the UL/OL level as the default for that instance. If you set a data-icon attribute on the LIs, that would override the icon set ont he UL/OL. I think this is what @ugomobi was suggesting in #2, but wanted to confirm.

Member

jaspermdegroot commented Sep 18, 2012

Actually I was suggesting that in # 3 :)

I modified my previous comment to clarify on which 3 levels the user can set the icon option.

Contributor

MauriceG commented Sep 18, 2012

Hi @ugomobi !

With the code above, it will not be possible to disable the icon at LI-level.

With my first PR for this feature it will ;-)

Maurice

Member

jaspermdegroot commented Sep 18, 2012

@MauriceG

Yes you are right. Just noticed that while testing my code. @jasondscott is working on this now.

Contributor

MauriceG commented Sep 18, 2012

Oh cool, thanks.

Jason Scott added some commits Sep 18, 2012

Jason Scott Listview: Renamed option to icon. Added ability to change icons from the
ul

This provides the ability for the follow cases:

1. set the option as default for all listviews using the $.mobile object
2. set (or override the default) icon with data-icon on a listview or with
the option icon if that listview is created programmatically
3. override the above for a list item with data-icon on that specific list
item
3c73552
Jason Scott Docs: Updated the listview docs for the icon option b27d601
Jason Scott Docs: Updated the data-attribue reference for listview icon option
Moved listview icon option to the correct place alphabetically in docs.
d73c9ef
Contributor

jasondscott commented Sep 18, 2012

@toddparker @ugomobi Can I pull this into master?

Contributor

toddparker commented Sep 18, 2012

I think I'd rather wait for a week since this is a new feature and we're at RC. As soon as 1.2 final is tagged, then yes. Looking forward to this addition.

Member

jaspermdegroot commented Sep 18, 2012

@jasondscott

Nice!

+1 for merging after 1.2 is out. If we land this in 1.2.1 let's not forget to pick in the 1.2-stable branch :)

jasondscott was assigned Sep 19, 2012

@jasondscott jasondscott added a commit that referenced this pull request Oct 9, 2012

@jasondscott jasondscott Merge pull request #5018 from jquery/listDefaultIcon
Listview: Added option for the default list icon
4a9427c

@jasondscott jasondscott merged commit 4a9427c into master Oct 9, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment