-
Notifications
You must be signed in to change notification settings - Fork 354
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
[guides] Finalize Guides for v2.0.0-beta.1 #549
Conversation
aae30f9
to
fb5b3a8
Compare
This PR updates the docs app for the first beta of v2. Changes include: - Converting all curly bracket invocations to angle brackets - Adding missing guides pages - Adding API docs - Shifting examples -> guides (better terminology) - Adding/updating intro and landing pages There is also one minor behavioral change, we've dropped the `CellProxy` construct. In writing the guides it became apparent that it was leaky, and the perf benefits are questionable. For now, cell metas are POJOs.
fb5b3a8
to
674300c
Compare
destroy() { | ||
this._cellMeta.destroy(); | ||
if (!cellMetaCache.has(cellId)) { | ||
cellMetaCache.set(cellId, EmberObject.create()); |
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.
do we ever teardown the cache properly?
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.
Yes, this is torn down by the owner which is the tbody
.
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.
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.
my C++ days make me always worried to not see a corresponding destroy
to a create
``` | ||
|
||
@yield {any} cellValue - The value of the cell | ||
@yield {object} columnValue - The column definition |
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.
was there discussions to use columnDefinition
instead of columnValue
?
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 don't think we ever discussed it. I would be open to that, but I'd like to get this published first and come back to it if we find it's unintuitive.
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.
that's fine.
|
||
cell=(component "ember-th" api=cell) | ||
enableResize=api.enableResize |
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.
some options here do not seem documented
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.
Yes, that was intentional. Parts of the API are public, and parts aren't really and are just for communication. In the future ideally we will figure out a way to make these more genuinely private.
related to a particular cell, column, or row. A good example of this is cell | ||
selection, like in Excel. | ||
|
||
When you click a cell in excel, the row, column, and cell are all marked as |
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.
Excel
|
||
An addon to support large data set and a number of features around table. Ember Table can handle over 100,000 rows without and rendering or performance issue. This version of Ember Table supports Ember 1.11 to latest version of Ember. | ||
Ember Table is a power table made for users who need a full-fledged, |
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.
are we not too "elitist" here? it seems like that it works pretty well out of the box, no?
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.
Not sure what you mean exactly here, can you clarify?
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.
By reading this paragraph, I have a feeling that Ember Table wouldn't be a great fit for people wanting a simple table.
}) | ||
rows = [ | ||
{ | ||
firstName: 'Cyril', |
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.
Is this a test to see if I'm doing the code review diligently? 😄
we can find a better example 😉
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.
lol, I just needed a first name. If you want me to change it I can, no worries
`column`. To customize table header or footer, you can pass in `headerComponent` and | ||
`footerComponent` fields in each column data. When the `headerComponent`(or `footerComponent`) is | ||
defined, the `name`(or `footerValue`) field is ignored. | ||
And viola! You should have a basic table up and running! |
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.
voila 🇫🇷
|
||
Building that functionality into your Single Page App is a long, tiresome | ||
process, and as the datasets scale it just gets harder. There are a lot of | ||
different off the shelf tables out there, like ag-grid and HandsOnTable, but |
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.
not sure if we want to reference other tables.
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 can remove the references. Mainly wanted to highlight the differences, and why we're better for many use cases.
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.
My main concern is that I don't know if this could feel outdated each time someone else try to publish a new addon.
0a63b77
to
5165279
Compare
This PR updates the docs app for the first beta of v2. Changes include:
There is also one minor behavioral change, we've dropped the
CellProxy
construct. In writing the guides it became apparent that it was leaky,
and the perf benefits are questionable. For now, cell metas are POJOs.