dj-fiorex / angular2-smart-table Goto Github PK

The "class" column setting is only applied to the column header. We should improve this feature by

1. changing the documentation to be more precise that this setting only affects the column header
2. adding another setting for the class of the column content cells

A possible solution could look like this:

classHeader → the css class for the header cell
classContent → the css class for the content cells
class → alias for classHeader for backwards compatibility

when clicking select all checkbox on selectionMode: 'multi', it only selects the rows currently displayed in the page not all the rows in the DataSource. Furthermore, on changing page or searching any column looses the selection

When using the editor of the type "list", I noticed that the first selection is always automatically set as the default when clicking in Edit. This means that the filter element that is currently set in the line is not preselected.
As far as I could test, this behaviour did not occur with ng2-smart-table.

When the user calls setSort() to reconfigure the sorting settings, the sort config object expects three fields: field, direction, and compare.

The field compare is not mandatory. Leaving it undefined however, causes the default comparator that has been configured via the column settings to be overwritten. This is very unexpected.

So either all three settings should be mandatory, or (and I guess that would be the better approach) an already configured comparator should be kept when the setSort() is invoked.

Make table configuration accepting a typescript interface to strongly typed rows data,
This might add some flexibility and confidence while loading data to the table, and also make it easier to manipulate data in the tables.

In the table settings you can specify the desired width of a column,

However, when you use table filters or the pagination, the widths of the columns automatically adjust and seem to ignore the predefined widths (at least the new widths do not seem to come even close to what was configured).

This bug is very tricky to fix, because in contrast to the original ng2-smart-table, this fork of the smart-table supports changing the column width via drag&drop. I.e. we have to respect two settings: 1) the original width from the settings object and 2) a possible user defined width that has been selected at runtime.

I did not quite understand how the column resizing works (and imho it's very laggy and unpredictable), so I cannot provide a solution idea just yet.

Hello dj-fiorex,

As described here, libraries should be compiled with enableIvy:false;
angular/angular#35291

I tried to migrate from ng2-smart-table to your fork, but had to rollback because of this issue.

Thanks for your help !

Edit : And of course thank-you for upgrading this library !

How to add the orignal style of the table like that in the ngx-demo-Smart Table?
I found this file: node_modules@nebular\theme\styles\global\tables_smart-table.theme.scss nebular github
But of course as this package name angular2-smart-table and not ng-smart-table as the original package the style not apply.

Any idea how to set the origianl style?

Sometimes, the sort direction for columns does not work. Clicking on the column starts sorting the values ascending, but clicking again does not change the sort order to descending.

I did not manage to analyze every detail, but there is something that already bugged me...

I think the settings key should not be sort but sortDirection. But I cannot tell if this has anything to do with the actual bug.

Update: The sort direction bug never existed. It was an error within a custom sort function of the analyzed program. But I keep this ticket still open as a reminder to look into the line quoted above.

Some things have changed compared to ng2-smart-table.

We should document (at least the most significant) changes in the README.md and give some advise for migration.

This is a manually maintained project without a dependency bot creating issues for us.

Therefore we must update the dependencies before releasing the next version.

For every row action (default and custom) we should add functions that map row data to a boolean determining whether the action is disabled or completely hidden. For the header actions (currently this is only the add action), the function does not take an input parameter.

Example settings:

actions: {
  add: true,
  edit: {
    hiddenWhen: (data) => !data.readonly
  },
  delete: {
    disabledWhen: (data) => data.used
  },
}

or

someExternalFlag = false;

actions: {
  add: {
    disabledWhen() => this.someExternalFlag
  }
  edit: false,
  delete: true,
}

For compatibility it is mandatory that the settings field keep their boolean type. So the new type would be boolean | ActionSettings.

relates to #82
relates to #81
resolves #2

The implementation always calls the method for single row selection:

This suggests that the feature only works reliable in 'single' selection mode. But it is yet unclear if there is really an observable bug.

At least the demo application seems to work just fine.

This method returns the number of columns plus the optional action column but forgets that there might be another optional column for multi-selection of rows.

selectMode: 'multi' adds a select all button on the top left, but it only selects rows in that are currently displayed. IMO it should select all the rows across all pages.

Compare this implementation:

with this:

I do not have a clue why the multi-select variant does not invoke the respective grid function and I do not know if this is a bug.

I'm not sure if there's a specific reason for it to be this way, but the ViewCell interface only allows string | number, but I have a complex object in my data. Ultimately, I'd like to be able to have a RenderComponent that is able to do this.renderedValue = this.value.fullName. It seems as easy as changing the ViewCell.value property to any. Is there a reason not to make this change?

Replicate the changes described in PR #5

Of course we cannot merge the PR because the selectors are already renamed, but we can replicate the changes to add cancel events.

However, we should use a better name for the cancel event.

In PR:

editConfirm / cancel
createConfirm / createCancel

Better:

editConfirm / editCancel
createConfirm / createCancel

The smart table can display HTML content in column cells.

But since angular does aggressively modify the HTML the developer intended to display, the result is often very unexpected. For example, it is not possible to use inline SVG content. It is not even possible to use custom inline styles (Angular would remove that).

Therefore, we should implement a pipe using the DomSanitizer to explicitly allow custom HTML or custom styles.

However, since this is apparently a security feature, we should not enable it by default. We would need new column settings for that (including the action column for which the user might want to use svg icons).

This fork introduced a new feature that allows resizing of columns cells.

The way this is achieve is to register a mouse event listener to the header cell and change the width of that cell (and it's sibling) according to the mouse offset when dragging the mouse.

However, the individual cell renders are not notified about the width change. This is problematic for cell renderers that use the width to constrain the contents (e.g. div containers that use text-overflow: ellipsis).

Currently, the cell renderers are usually tied to the width configured in the settings object, which should be the initial width of the column (which is also not always true, see #35 ).

There should be a reliable (and preferably cost efficient) way to communicate width changes to cell renderers.

We check compatibility with angular 14, but leave the minimum required version to 13.

At least as long as the "long" term support of Angular 13 lasts (May, 4th next year).

Since the ServerDataSource does not implement getAll(), the data returned by getAll() is the data that was last retrieved from the server (so usually the data that would be returned by getElements().

A possible implementation could be to submit a request without query parameters.

@hexdecimal16 @dj-fiorex

This push operation can add ghost rows to the table, e.g. when sorting or inline editing. Maybe there are more operations that lead to this incorrect behavior. We should fix this asap, because this is a critical bug.

Please, in the future, do not merge (and more importantly do not release) untested changes that quickly. I know this whole npm community has a release first, fix second mentality, but since this table may be used in production code by many people, we should be more careful.

Please add Angular 14 to your peer dependencies. Currently, I can only compile by using npm install --force or by declaring overrides in my package.json. The updated version of your package.json will look something like this:

  "peerDependencies": {
    "@angular/common": "^13.0.0 || ^ 14.0.0",
    "@angular/core": "^13.0.0 || ^ 14.0.0",
    "@angular/forms": "^13.0.0 || ^ 14.0.0",
    "angular2-multiselect-dropdown": "^5.0.4",
    "lodash-es": "^4.17.21",
    "ng2-completer": "^9.0.1"
  },

Thanks in advance,
Stephan

How to know when the user has clicked select all button in selectMode: 'multi'

There are calls into deprecated rxjs functions (e.g. toPromoise()) that will be removed in rxjs v8.

We should start replacing them.

It would be very interesting if you could specify in the global table settings that the header rows can be sticky. Especially with larger data sets or more data sets per page, you otherwise have to constantly scroll upwards if you have many columns in a table.

First I thank you,
For such great initiative to create fork of ng2-smart-table.

Motivation

I'm trying to get page number on click of a page number of pagination links.

feature request

Can we add a custom event so on click of pagination, we can get page number for further processing.

Usability :
For some scenario if we need page number to prepare data for next page with page number
For example if user is on page 1 then app need to prepare data for page 2 and page 3, ...

This will be useful if we can add support for that.

Maybe this is a bug, or maybe this is just a change that needs to be documented in the migration guide.

The observation is that in the original smart table implementation a click on the "select all" checkbox in the column header of the selection column fired the select events for the individual items.

The current implementation does not.

This might be a reasonable change, but it is also a breaking change. Need to investigate.

It seems there was a feature hacked into the filter configuration that allows to combine filters with OR instead of AND.

This feature is, however, very incomplete for the following reasons:

1. It is not implemented for ServerDataSource
2. The filter implementation is expensive (it applies each field individually as AND filter with one criterion and then merges the results and tries to get rid of duplicates afterwards instead of properly implementing the OR operator)
3. It is not possible to combine AND and OR (despite the addFilter signature might suggest that)
4. The default filter row always reverts the operator back to AND when used

Improving or removing this feature would be both breaking changes. So this issue here is just a reminder that something should be done about this in case a major upgrade is planned eventually.

This feature is a bit dependent of #68 because it only makes sense when the table has fixed table layout.

The goal is to implement text-overflow: ellipsis as default but let the user decide which overflow behavior they want.

This makes it unnecessary for users to implement a custom renderer for such a simple task.

However, there are several more things to consider here, e.g. the feature for adjusting column widths.

We would need careful testing to check if this feature works in several different scenarios.

The setting perPageSelect is not documented and also missing in the settings interface (the former is a documentation issue and the latter is considered a bug).

The feature itself also has two bugs:

1. The selector is usually displayed right-aligned below the table. But when the page selection is hidden (e.g. because there is only one page to show), the selector is displayed left-aligned and consumes the space where the page selection should be.
2. The margin of the perPageSelect does not equal the margin of the page selection

Additionally, we should make the label optional or at least let the user choose a label text.

The column containing the checkboxes for a multi-selection does not have a specific width.

That means it can fill the remaining width of the table and become ridiculously large.

Bad:

Good:

using serverDatasource pagination not working

this.source = new ServerDataSource(this._http, {
      endPoint: `${environment.baseUrl}/admin/users`,
      dataKey: "data",
      pagerPageKey: "page",
      pagerLimitKey: "pageSize",
      totalKey: "total",
    });

The update to the sorting configuration (see #39) resulted in a strongly typed configuration array.

We should also do this for the filter configuration.

Currently the smart table does not really respect column width settings. You decide that the column shall have width 8rem for example but the true width is not 8rem but some weird mixture of what you "suggested" for the column width and what the table thinks is best.

Another problem is that the sum of column widths (not the widths you have chosen, but the widths the table thinks are best) may exceed the total width of the table. And that results in the table having more actual width than it should have.

As a possible fix we might add table-layout: fixed to the style of the table element.

If I understand correctly, the intention of the original authors was to control the column widths via the widths of the header cells anyway. So a fixed table-layout would be the natural choice here.

ComponentFactoryResolver is deprecated since Angular 13.

Remove all uses.

Consider this code:

this.isSortable = typeof this.settings['sort'] === 'undefined' ? true : !!this.settings['sort'];
this.isEditable = typeof this.settings['isEditable'] === 'undefined' ? true : !!this.settings['isEditable'];

We should either

• rename both settings and include that change in the migration guide (#29 )
• revert the change for editable (the original name was editable)

Hi,

just switched from ng2-smart-table to your fork. This is the result:

The rows background color is the same for all and the pager is not aligned to center.

Just write to know if I have a enviroment issues or is ok.

Regards

I've prepared a pull request updating the library to Angular 13.

The demo page contains information to get into contact with "us", which links to akveo Twitter / Facebook etc. We should convert that into a proper Credits Section.

There is an example for custom actions, but no documentation of the settings.

Relates to #81

The CustomAction interface is incomplete (missing renderComponent).

The method getFilteredAndSorted() is documented in the HTML documentation and implemented in the LocalDataSource, but it is neither part of the DataSource interface, nor implemented in the ServerDataSource.

Consider the following two calls on a LocalDataSource:

this.source.load([]).then();  // smart table automatically refreshes
this.source.empty().then(); // smart table does not refresh

Workaround:

this.source.empty().then(() => this.source.refresh());

Conjecture:

The base class DataSource emits onChanged events in both cases, but with different actions. My guess is that the smart table reacts on an onChanged event with action load (and refresh of course) but does not react on an onChanged event with action empty.

When a column contains null values, the default sort function gets confused. I guess the reason is that null values are neither strictly less nor strictly greater than any non-null value.

The count() method of the LocalDataSource returns the number of filtered and sorted elements (if they are already prepared) and not the total number of elements as specified by the documentation.

The ServerDataSource e.g. does not have this behavior (it returns the total number of elements last responded by the server).

Fixing this might be tricky, because there may be people relying on this bug already.

First,
Thanks for working on this, I just stumbled across ng2 smart table and was very disappointed to see it abandoned.

The github pages doesn't seem to be loading for me, am I having an issue or ?

cellValue.toString() will blow up when cellValue is null.