-
Notifications
You must be signed in to change notification settings - Fork 25
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
Split state and record for a list item, fixed #129 #130
Changes from 7 commits
6a7d59e
74c01e5
c90bcdf
9462e67
9e6e12b
37c3b45
e008455
f403e20
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,44 +1,31 @@ | ||
{{! Template for the frost-list-content-container component }} | ||
{{#vertical-collection | ||
alwaysRemeasure=true | ||
alwaysUseDefaultHeight=alwaysUseDefaultHeight | ||
bufferSize=bufferSize | ||
content=items | ||
defaultHeight=defaultHeight | ||
firstReached=onLoadPrevious | ||
lastReached=onLoadNext | ||
scrollPosition=scrollTop | ||
useContentProxy=false | ||
|
||
{{#if pagination}} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it necessary to remove the "basic" {{#each}} loop in the pagination case? I'd prefer to not introduce {{#vertical-collection}} for those people using the pagination scenario in a minor release There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can remove it. I thought you asked me to replace the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I'd like to keep the existing pagination so that we don't risk breaking There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ping me when that's done and I'll merge There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure I reverted this :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, still appears as removed in the PR |
||
{{#frost-scroll | ||
hook=(concat hookPrefix '-scroll') | ||
}} | ||
{{#each items as |model index|}} | ||
{{yield model index}} | ||
{{else}} | ||
{{yield to="inverse"}} | ||
{{/each}} | ||
{{/frost-scroll}} | ||
{{else}} | ||
{{#vertical-collection | ||
alwaysRemeasure=true | ||
alwaysUseDefaultHeight=alwaysUseDefaultHeight | ||
bufferSize=bufferSize | ||
content=items | ||
defaultHeight=defaultHeight | ||
firstReached=onLoadPrevious | ||
lastReached=onLoadNext | ||
scrollPosition=scrollTop | ||
useContentProxy=false | ||
|
||
key='@identity' | ||
resizeDebounce=64 | ||
firstVisibleChanged=null | ||
lastVisibleChanged=null | ||
didMountCollection=null | ||
idForFirstItem=null | ||
renderFromLast=false | ||
renderAllInitially=false | ||
shouldRender=true | ||
minimumMovement=15 | ||
containerSelector=null | ||
containerHeight=null | ||
key='@identity' | ||
resizeDebounce=64 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 64? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure what this is. I simply used the same vertical collection as below... I feel like we have a lot of properties that might not be necessary in the vertical collection. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, that's fine - it's like 15 FPSish - just an odd default, but no problem |
||
firstVisibleChanged=null | ||
lastVisibleChanged=null | ||
didMountCollection=null | ||
idForFirstItem=null | ||
renderFromLast=false | ||
renderAllInitially=false | ||
shouldRender=true | ||
minimumMovement=15 | ||
containerSelector=null | ||
containerHeight=null | ||
|
||
as |model index| | ||
}} | ||
{{yield model index}} | ||
{{else}} | ||
{{yield to="inverse"}} | ||
{{/vertical-collection}} | ||
{{/if}} | ||
as |model index| | ||
}} | ||
{{yield model index}} | ||
{{else}} | ||
{{yield to="inverse"}} | ||
{{/vertical-collection}} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,31 @@ | ||
const blueprintHelper = require('ember-frost-core/blueprint-helper') | ||
|
||
module.exports = { | ||
afterInstall: function () { | ||
return this.addAddonsToProject({ | ||
packages: [ | ||
{name: 'ember-frost-core', target: '^1.5.1'}, | ||
{name: 'ember-frost-sort', target: '^7.0.0'}, | ||
{name: 'ember-math-helpers', target: '^2.0.0'}, | ||
{name: 'smoke-and-mirrors', target: '~0.6.2'} | ||
] | ||
afterInstall: function (options) { | ||
const addonsToAdd = [ | ||
{name: 'ember-frost-core', target: '^1.14.3'}, | ||
{name: 'ember-frost-sort', target: '^7.0.0'}, | ||
{name: 'ember-math-helpers', target: '^2.0.5'}, | ||
{name: 'smoke-and-mirrors', target: 'github:ciena-frost/smoke-and-mirrors'} | ||
] | ||
|
||
// Get the packages installed in the consumer app/addon. Packages that are already installed in the consumer within | ||
// the required semver range will not be re-installed or have blueprints re-run. | ||
const consumerPackages = blueprintHelper.consumer.getPackages(options) | ||
|
||
// Get the packages to install (not already installed) from a list of potential packages | ||
return blueprintHelper.packageHandler.getPkgsToInstall(addonsToAdd, consumerPackages).then((pkgsToInstall) => { | ||
if (pkgsToInstall.length !== 0) { | ||
// Call the blueprint hook | ||
return this.addAddonsToProject({ | ||
packages: pkgsToInstall | ||
}) | ||
} | ||
}) | ||
}, | ||
|
||
normalizeEntityName: function () { | ||
// this prevents an error when the entityName is | ||
// not specified (since that doesn't actually matter to us) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry - can you put a quick explanation on the removal of this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need the frost scroll with the
vertical-collection
. The vertical collection has the scrolling capability.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reverted this 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you push the changeset? It still appears as removed in the PR