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

ion-list: upcoming 1.0.0 syntax #1024

Closed
wants to merge 2 commits into from

Conversation

ajoslin
Copy link
Contributor

@ajoslin ajoslin commented Apr 2, 2014

List refactor:

  • Declarative syntax
  • More flexibility
  • Performance increase for scrolling - will only make the DOM for the item more complex as elements are added (eg delete button section only if delete button is added)

Simple case:

<ion-list>
  <ion-item ng-repeat="i in items">Hello!</ion-item>
</ion-list>

Everything case:

<ion-list show-reorder="shouldReorder" 
  show-delete="shouldDelete"
  can-swipe="swipeIsAllowed">
  <ion-item ng-repeat="i in items" on-reorder="onReorder()">
    {{i}}
    <ion-option-button class="button-positive">Hello!</ion-option-button>
    <ion-option-button class="button-assertive">Goodbye!</ion-option-button>
    <ion-delete-button ng-click="items.splice($index, 1)" class="ion-minus-circled"></ion-delete-button>
    <ion-reorder-button class="ion-navicon"></ion-reorder-button>
  </ion-item>
</ion-list>

Thoughts, questions, concerns, complaints, insults, death threats?

/cc @adamdbradley @mlynch @mhartington @CoenWarmer @rvanbaalen @D3CK3R @calendee and anyone else who has a comment

@ajoslin ajoslin changed the title ion-list: upcoming 1.0.0-beta.2 syntax ion-list: upcoming 1.0.0 syntax Apr 2, 2014
@adamdbradley
Copy link
Contributor

What does the final markup look like when everything is used?

I do like the idea of item-content not being added if there are no option or delete buttons, that'll be nice to reduce the elements. But the item directive is already using ng-if for options and deletes, they are not always added to the dom as it stands.

Right now item-content is currently always an <a> element. It'd be nice to only have it an anchor when there was an href assigned.

Also, when ng-click is assigned to the directive, it's assigning it to the entire item, but that's not ideal when there are things like optional buttons and deletes. Can you look into having ng-click only assigned to item-content and not the entire row? Right now there are pointer-event tricks so not everything is clickable.

@ajoslin
Copy link
Contributor Author

ajoslin commented Apr 2, 2014

  1. ngIf and ng-repeat for the option buttons add additional nodes and processing, even if they are blank (it's probably negligible when compiling, but when scrolling I bet it could reduce perf). I think this will be quicker to compile and scroll.
  2. I did make it so it's only an <a> if there's a href or ngHref present
  3. Good idea about the click, I'll see if it's doable

One more change I forgot to list: delete buttons and reorder buttons are actually display: none instead of just opacity 0 while not used now, which will definitely increase scrolling perf

@CoenWarmer
Copy link

I think this is great @ajoslin. I see very little reason for death threats this time around. ;)

Performance increases sound very good.

Perhaps some things to consider for a next version are sticky list headers (yup I had to mention them) and perhaps even inclusion of bindonce if dirty checking isn't needed. But I heard you were also thinking of implementing virtual scroll in the next big release so that might cover the reason for using bindonce in the first place.

@CoenWarmer
Copy link

Also: how can we set if an item is a divider?

@ajoslin
Copy link
Contributor Author

ajoslin commented Apr 2, 2014

  1. Sticky items (I think just sticky elements in general) would be good to have
  2. Virtual scroll is a yes! Starting work on it very soon
  3. type is now just a class name. I'll be sure to give adequate examples in the docs.

@CoenWarmer
Copy link

Looking forward to it!

@mhartington
Copy link
Contributor

@ajoslin

I did make it so it's only an if there's a href or ngHref presen

Will this work if you use ui-routers ui-sref?

And +1 for sticky item headers

@ajoslin
Copy link
Contributor Author

ajoslin commented Apr 2, 2014

Yes, normal click events on a div will work for everything except hrefs.

@mlynch
Copy link
Contributor

mlynch commented Apr 2, 2014

I have an old branch with a version of sticky headers kind of implemented (needs polish).

Maybe we could have two options for that:

Or on item:

Cats

We can move that to a different discussion issue just got all excited :)

Sent from my iPhone

On Apr 2, 2014, at 9:21 AM, Andy Joslin notifications@github.com wrote:

Yes, normal click events on a div will work for everything except hrefs.


Reply to this email directly or view it on GitHub.

@ajoslin
Copy link
Contributor Author

ajoslin commented Apr 2, 2014

@mlynch We could also consider having a generic 'sticky' option to apply to any old element.

@mhartington
Copy link
Contributor

@ajoslin I like that idea better. Though I'm not sure what other situations where a sticky element could be used except lists, it's better to abstract it so it could implemented anywhere

@ajoslin
Copy link
Contributor Author

ajoslin commented Apr 2, 2014

Also, $ionicListDelegate is coming :-)

  • .showReorder(boolean) (syncs with ionList directive attribute)
  • .showDelete(boolean) (syncs with ionList directive attribute)
  • .canSwipeItems(boolean) (syncs with ionList directive attribute)
  • .closeOptionButtons()

Any other useful methods to add?

@mhartington
Copy link
Contributor

.swipeDirection() ?
I've seen a few people on the forum asking for how to change the swipe direction, mostly for RTL

@mlynch
Copy link
Contributor

mlynch commented Apr 2, 2014

@ajoslin, maybe there's room for two? For a list, you often generate a group for quick sorting (same goes for that collection view stuff we talked about). The tricky part is you are moving multiple stickies in and out (each group header "pushes" the last one up, for example)

A sticky to me is just a way to make position: sticky work with our scroll system, which would be totally awesome but maybe two separate things?

@ajoslin
Copy link
Contributor Author

ajoslin commented Apr 2, 2014

Good point, you do usually do groups. I'll get this initial version pushed out, let's open a new issue for stickies. #1025

@ajoslin ajoslin mentioned this pull request Apr 2, 2014
@adamdbradley
Copy link
Contributor

@ajoslin Oh, and be sure to test the animations of the delete and reorder icons when they are first animate in. That's they're start out using opacity: 0.

@ajoslin
Copy link
Contributor Author

ajoslin commented Apr 2, 2014

👍

@ajoslin
Copy link
Contributor Author

ajoslin commented Apr 2, 2014

Added a PR. Still TODO:

  • Get some code review action (anyone and everyone who wants to, please berate and insult and criticize the code in the PR, it's important list is done right)
  • Add breaking changes
  • Update codepens/demos
  • Update internal html tests

@calendee
Copy link

calendee commented Apr 2, 2014

Also need some method to close the swiped list item. That's been requested
several times on the forum.

User swipes to expose buttons. Clicks something. List should
automatically close or give the developer an API to close it after some
action completed.

On Wed, Apr 2, 2014 at 4:24 PM, Andy Joslin notifications@github.comwrote:

Added a PR. Still TODO:

  • Get some code review action
  • Add breaking changes
  • Update codepens/demos
  • Update internal tests

Reply to this email directly or view it on GitHubhttps://github.com//pull/1024#issuecomment-39385722
.

Thanks,
Justin Noel
http://kidsintouch.com
http://calendee.com
@calendee
@kidsintouch

@ajoslin
Copy link
Contributor Author

ajoslin commented Apr 3, 2014

@calendee $ionicListDelegate.closeOptionButtons() is available with the change 👍

BREAKING CHANGE: ion-list syntax has changed in favor of simplicity &
flexibility.

Relevant documentation:
[ionList](http://ionicframework.com/docs/api/directive/ionList),
[ionItem](http://ionicframework.com/docs/api/directive/ionItem),
[ionOptionButton](http://ionicframework.com/docs/api/directive/ionOptionButton),
[ionReorderButton](http://ionicframework.com/docs/api/directive/ionReorderButton),
[ionDeleteButton](http://ionicframework.com/docs/api/directive/ionDeleteButton),
[$ionicListDelegate](http://ionicframework.com/docs/api/service/$ionicListDelegate).

To migrate, change your code from this:

```html
<ion-list option-buttons="[{text:'hello',type:'button-positive',onTap:tap()}]"
          on-delete="onDelete(el)"
          delete-icon="ion-minus-circled"
          can-delete="true"
          show-delete="shouldShowDelete"
          on-reorder="onReorder(el, startIndex, toIndex)"
          reorder-icon="ion-navicon"
          can-reorder="true"
          show-reorder="shouldShowReorder">
  <ion-item ng-repeat="item in items">
    {{item}}
  </ion-item>
</ion-list>
```

To this:

```html
<ion-list show-delete="shouldShowDelete"
          show-reorder="shouldShowReorder">
  <ion-item ng-repeat="item in items">
    {{item}}
    <ion-delete-button class="ion-minus-circled"
                       ng-click="onDelete(item)">
    </ion-delete-button>
    <ion-reorder-button class="ion-navicon"
                       ng-click="onReorder(item, $fromIndex, $toIndex)">
    </ion-reorder-button>
    <ion-option-button class="button-positive" ng-click="tap()">
      Hello
    </ion-option-button>
  </ion-item>
</ion-list>
```
@ajoslin ajoslin closed this in 986dbac Apr 4, 2014
@ajoslin
Copy link
Contributor Author

ajoslin commented Apr 4, 2014

Merged!

@leandroz
Copy link

leandroz commented Dec 2, 2014

@ajoslin

.showOptionButtons(boolean)

maybe? I want the same behavior of WhatsApp, when you click on Edit and then in the delete button, it shows the option button to actually erase the message.

@argyleink
Copy link

I would like to the ability to programmatically open and close list item drawers. For example, Android should be able to listen for a long tap, and invoke the opening of the option buttons for that applicable list item. Thanks for all the rad work!

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.

8 participants