Closes ciena-frost/ember-frost-list#121 #122
Conversation
|
Changes Unknown when pulling 920201e on notmessenger:is-loading-indicator into ** on ciena-frost:master**. |
|
Changes Unknown when pulling 854db7b on notmessenger:is-loading-indicator into ** on ciena-frost:master**. |
| itemExpansion: PropTypes.EmberComponent, | ||
| loadingType: PropTypes.string, |
There was a problem hiding this comment.
I'd like this to be change to a component closure rather than relying directly on frost-loading
So the interface would be
{{frost-list
loading=(component 'frost-loading')
...
}}
| {{#if pagination}} | ||
| {{#frost-scroll | ||
| hook=(concat hookPrefix '-scroll') | ||
| {{#if isLoading}} |
There was a problem hiding this comment.
Can this be satisfied using the inverse block of the #each?
| @@ -15,8 +15,11 @@ | |||
| <div class='frost-modal-demo-selector-title'> | |||
| Cookbook | |||
| {{#link-to 'simple'}}Basic{{/link-to}} | |||
| {{#link-to 'simple-loading'}}Basic - Loading{{/link-to}} | |||
There was a problem hiding this comment.
You can put the examples directly into the existing routes - no need to do a duplicate explicitly to demonstrate the loading
There was a problem hiding this comment.
@sglanzer The demo app currently uses small viewports for the existing demos so that users don't have to scroll as far to experience the infinite paging experience, not as many records need to be created for the paging screens, etc. Because this viewport is not very tall (due to it's overflow being hidden) the loading indicator does not show in its entirety. In the new routes I created to demo the loading support I have increased the viewport size. If you have any recommendations on how to remedy this or approach it differently I am interested. In either case I am in need of direction on how you wish me to proceed.
There was a problem hiding this comment.
Ah, I don't think we made a conscious decision about the demo height initially, if you can increase it and satisfy your case of showing the loader I'm fine with that. I believe it's easier to use the demo for a reference if you don't have to go through multiple demos to get the data you need (less is more).
Not a big deal though, so if you see value in keeping both I won't block it
|
@sglanzer Regarding the exploration of the explicit need for |
|
@notmessenger ok, let's avoid that for now and stick with the |
|
@sglanzer Or we could have it also accept a |
|
Sure, but an explicit isLoading still won't hurt in that case, so I'm fine with this, just need the adjustment to the way the loading indicator is specified and I'm good to merge |
This project uses semver, please check the scope of this pr:
CHANGELOG
frost-list