-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
docs(angular-table) Angular table state docs #5545
docs(angular-table) Angular table state docs #5545
Conversation
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 4182c74. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
@KevinVandy @merto20 About this, I was thinking of a solution that could be implemented but may require some effort. Example: notifier = signal({});
mergeOptions(prevOptions, nextOptions) {
const mergedOptions = {...prevOptions};
for (const property in nextOptions) {
const getter = Object.getOwnPropertyDescriptor(nextOptions, property);
if (!getter) {
Object.defineProperty(mergedOptions, property, { value : nextOptions[property] };
} else {
Object.defineProperty(mergedOptions, property, {
get() {
notifier.set({});
return getter();
}
})
}
}
return mergedOptions;
} This example is simplified, doing a ready implementation like the vue/solid one (which uses solid-js mergeOptions) may require some time and could cause more issues than it solves. Considering that we currently can handle the fully-controlled state via signals during the initialization, in my opinion we don't necessarily have to support this now and we may can look into it after the release, get feedback from consumers and then evaluate whether it makes sense to do . Following the other framework approach, we should support something like this: class Component {
readonly table = createAngularTable(() => {});
readonly state = signal({...table.initialState})
constructor() {
const {state} = this;
effect(() => {
table.setOptions(prev => ({
...prev,
get state() {
return state();
}
})
})
}
} |
We should eventually just make the "fully controlled" example like the react one to go along with these docs. Our docs can evolve. |
* feat(angular-table): adds Angular adapter for tanstack/table (#5326) * Angular adapter for tanstack table initial * build with: ng packagr * Add angular examples basic grouping * Add angular examples basic grouping * Update angular examples basic grouping * Add angular example selection * regen lock file * package upgrades, angular table docs * prettier * move around some deps * removed unused dependency from angular package * fix deps --------- Co-authored-by: Kevin Van Cott <[email protected]> * docs config cleanup * feat: signal angular table adapter implementation * update demo * feat: table proxy detect memoized fns * fix proxy property returning value * feat: improve naming * save new reference of table signal on every update * computed trap proxy for fns with 1 argument * update pnpm-lock.yaml * refactor proxy implementation * fix dependencies * cleanup imports * refactor basic example * fix build * run prettier * add grouping example, fix ci * add grouping example, fix ci * add row selection example * add column visibility example * update examples add signal input required example * improve angular table impl, fix flex-render change detection issues * fix build * feat(angular-table): improve adapter implementation (#5524) * feat(angular-table): support render dynamic components and templateRefs in table * update row selection example using dynamic rendered components * support change detection on push with flex render * add column ordering example * fix flexRender change detection issues * rename properties * fix prettier and adjust example budget options * update basic example * add again support for table signal * add column-pinning example * add column pinning example * add filters example * cleanup code * example cleanup * update lock file * feat(angular-table): added injector optional parameter for more flexibility (#5525) * feat(angular-table): improve adapter implementation (#5524) * Added optional injector for more flexibility * Replace runInInjectionContext with injector in effect * feat(angular-table): Added proxifyTable back * feat(angular-table): adding back notifier signal for table changed * feat(angular-table): Improve logic in setting table options *set table options inside computed before returning the table instance *remove redundant signals and effect *remove injector as it no longer required *update Grouping example to show how to pass signal when creating table * update angular adapter and state docs * prettier * update docs config with angular examples * update angular table state docs (#5545) * Angular examples and dependencies improvements (#5546) * tslib should be a dependency, see: https://angular.io/guide/angular-package-format#tslib * ensure angular examples are run as web container on code sandbox * update lock file --------- Co-authored-by: Kevin Vandy <[email protected]> * docs(angular-table): Add documentation for FlexRenderDirective (#5543) * add flexRender documentation * fix docs * fix rendering component section heading * remove double build package.json from angular build * update link name * docs pass * update esm exports in package.json * update flexrender types --------- Co-authored-by: jrgokavalsa <[email protected]> Co-authored-by: riccardoperra <[email protected]> Co-authored-by: Riccardo Perra <[email protected]> Co-authored-by: mamerto-g <[email protected]> Co-authored-by: Arnoud <[email protected]>
@KevinVandy @riccardoperra Is it sensible to convert all documentation and examples to utilize signals? I am considering this if it proves to be a logical step. |
Isn't the doc already covering examples using signals? |
I would re-write the example to this: class TableComponent {
readonly pagination = signal<PaginationState>({ pageIndex: 0, pageSize: 15 })
tableOptions = computed(() => ({
data: this.data(),
columns: columns,
state: {
pagination: this.pagination(),
},
}))
table = createAngularTable(this.tableOptions)
constructor() {}
onNextPage() {
const pagination = this.pagination()
if (pagination.pageIndex < pagination.pageSize) {
pagination.pageIndex++
this.pagination.set({ ...pagination })
}
}
onPrevPage() {
const pagination = this.pagination()
if (pagination.pageIndex > 0) {
pagination.pageIndex--
this.pagination.set({ ...pagination })
}
}
} Passing the |
You use onStateChange/state in order to have a single object with the entire state of the table, which allows you to have a full state controlled approach. We already have an example to individually handle the state (https://tanstack.com/table/latest/docs/framework/angular/guide/table-state#individual-controlled-state) which is ported from the other adapters. Also, you may want to use next/back methods from tanstack table to update your signals in order to have an already working implementation to update the state instead of re-writing another one from scratch, so you'll end up to use the onStateChange/onPaginationChange handlers to synchronize them. |
I specifically mention We are always re-writing the table options object, that's the only way we notify the table instance to update the options internally. The I use |
I've updated the table state docs according to the angular implementation.
I had to refactor a bit the example of the
Individual Controlled State
combining rxjs and signals instead of usingangular-query
, but I'm not sure about that. It's certainly a good way to make Angular Query known, but at the moment it's a library with the experimental flag. I didn't have any issue using it but I don't know if everything is clear to a user who sees this documentationI would like to get also some feedback from @merto20 about my
fully controlled state
approach, which in my opinion could differ from other implementations due to some constraints.This is what the other frameworks does:
This is what I did
In angular, we may have to pass both
state
andonStateChange
properties into thecreateAngularTable
initialization. UsingsetOptions
that introduces a new signal into the dependency graph breaks our rules to mark the view dirty. So the signal will be updated outside, but it's not reflected into the view and theproxified
properties